e6dfe8
From 7164bcc8bec5165de244a74f32ca1469c7091d2d Mon Sep 17 00:00:00 2001
e6dfe8
Message-Id: <7164bcc8bec5165de244a74f32ca1469c7091d2d@dist-git>
e6dfe8
From: Jiri Denemark <jdenemar@redhat.com>
e6dfe8
Date: Thu, 22 Feb 2018 13:30:27 +0100
e6dfe8
Subject: [PATCH] Pass oldDev to virDomainDefCompatibleDevice on device update
e6dfe8
MIME-Version: 1.0
e6dfe8
Content-Type: text/plain; charset=UTF-8
e6dfe8
Content-Transfer-Encoding: 8bit
e6dfe8
e6dfe8
When calling virDomainDefCompatibleDevice to check a new device during
e6dfe8
device update, we need to pass the original device which is going to be
e6dfe8
updated in addition to the new device. Otherwise, the function can
e6dfe8
report false conflicts.
e6dfe8
e6dfe8
The new argument is currently ignored by virDomainDefCompatibleDevice,
e6dfe8
but this will change in the following patch.
e6dfe8
e6dfe8
https://bugzilla.redhat.com/show_bug.cgi?id=1546971
e6dfe8
e6dfe8
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
e6dfe8
(cherry picked from commit b6a264e8550d4add3946ec2fd9ae31a76fbf16fe)
e6dfe8
e6dfe8
https://bugzilla.redhat.com/show_bug.cgi?id=1557922
e6dfe8
e6dfe8
Conflicts:
e6dfe8
	src/qemu/qemu_driver.c -- context, qemuDomainUpdateDeviceLive
e6dfe8
            requires connection pointer in RHEL
e6dfe8
e6dfe8
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
e6dfe8
Reviewed-by: Ján Tomko <jtomko@redhat.com>
e6dfe8
---
e6dfe8
 src/conf/domain_conf.c |  3 ++-
e6dfe8
 src/conf/domain_conf.h |  3 ++-
e6dfe8
 src/lxc/lxc_driver.c   | 15 ++++++++-----
e6dfe8
 src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++---------
e6dfe8
 4 files changed, 54 insertions(+), 18 deletions(-)
e6dfe8
e6dfe8
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
e6dfe8
index 031d4b8e55..e9bba70057 100644
e6dfe8
--- a/src/conf/domain_conf.c
e6dfe8
+++ b/src/conf/domain_conf.c
e6dfe8
@@ -27185,7 +27185,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
e6dfe8
 
e6dfe8
 int
e6dfe8
 virDomainDefCompatibleDevice(virDomainDefPtr def,
e6dfe8
-                             virDomainDeviceDefPtr dev)
e6dfe8
+                             virDomainDeviceDefPtr dev,
e6dfe8
+                             virDomainDeviceDefPtr oldDev ATTRIBUTE_UNUSED)
e6dfe8
 {
e6dfe8
     virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
e6dfe8
 
e6dfe8
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
e6dfe8
index 3817887322..fb09496e89 100644
e6dfe8
--- a/src/conf/domain_conf.h
e6dfe8
+++ b/src/conf/domain_conf.h
e6dfe8
@@ -3006,7 +3006,8 @@ typedef enum {
e6dfe8
 } virDomainDeviceAction;
e6dfe8
 
e6dfe8
 int virDomainDefCompatibleDevice(virDomainDefPtr def,
e6dfe8
-                                 virDomainDeviceDefPtr dev);
e6dfe8
+                                 virDomainDeviceDefPtr dev,
e6dfe8
+                                 virDomainDeviceDefPtr oldDev);
e6dfe8
 
e6dfe8
 void virDomainRNGDefFree(virDomainRNGDefPtr def);
e6dfe8
 
e6dfe8
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
e6dfe8
index 7829ef8618..fe2405d08b 100644
e6dfe8
--- a/src/lxc/lxc_driver.c
e6dfe8
+++ b/src/lxc/lxc_driver.c
e6dfe8
@@ -3579,6 +3579,7 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
 {
e6dfe8
     int ret = -1;
e6dfe8
     virDomainNetDefPtr net;
e6dfe8
+    virDomainDeviceDef oldDev = { .type = dev->type };
e6dfe8
     int idx;
e6dfe8
 
e6dfe8
     switch (dev->type) {
e6dfe8
@@ -3587,8 +3588,11 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
         if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
e6dfe8
             goto cleanup;
e6dfe8
 
e6dfe8
-        virDomainNetDefFree(vmdef->nets[idx]);
e6dfe8
+        oldDev.data.net = vmdef->nets[idx];
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
e6dfe8
+            return -1;
e6dfe8
 
e6dfe8
+        virDomainNetDefFree(vmdef->nets[idx]);
e6dfe8
         vmdef->nets[idx] = net;
e6dfe8
         dev->data.net = NULL;
e6dfe8
         ret = 0;
e6dfe8
@@ -4791,7 +4795,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
e6dfe8
         if (!vmdef)
e6dfe8
             goto endjob;
e6dfe8
 
e6dfe8
-        if (virDomainDefCompatibleDevice(vmdef, dev) < 0)
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
e6dfe8
             goto endjob;
e6dfe8
 
e6dfe8
         if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
e6dfe8
@@ -4799,7 +4803,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
e6dfe8
     }
e6dfe8
 
e6dfe8
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
e6dfe8
-        if (virDomainDefCompatibleDevice(vm->def, dev_copy) < 0)
e6dfe8
+        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
e6dfe8
             goto endjob;
e6dfe8
 
e6dfe8
         if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) < 0)
e6dfe8
@@ -4902,9 +4906,8 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
e6dfe8
         if (!vmdef)
e6dfe8
             goto endjob;
e6dfe8
 
e6dfe8
-        if (virDomainDefCompatibleDevice(vmdef, dev) < 0)
e6dfe8
-            goto endjob;
e6dfe8
-
e6dfe8
+        /* virDomainDefCompatibleDevice call is delayed until we know the
e6dfe8
+         * device we're going to update. */
e6dfe8
         if ((ret = lxcDomainUpdateDeviceConfig(vmdef, dev)) < 0)
e6dfe8
             goto endjob;
e6dfe8
     }
e6dfe8
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
e6dfe8
index b8022a06ba..3bd2d983a2 100644
e6dfe8
--- a/src/qemu/qemu_driver.c
e6dfe8
+++ b/src/qemu/qemu_driver.c
e6dfe8
@@ -7830,6 +7830,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
e6dfe8
 {
e6dfe8
     virDomainDiskDefPtr disk = dev->data.disk;
e6dfe8
     virDomainDiskDefPtr orig_disk = NULL;
e6dfe8
+    virDomainDeviceDef oldDev = { .type = dev->type };
e6dfe8
     int ret = -1;
e6dfe8
 
e6dfe8
     if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
e6dfe8
@@ -7847,6 +7848,10 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
e6dfe8
         goto cleanup;
e6dfe8
     }
e6dfe8
 
e6dfe8
+    oldDev.data.disk = orig_disk;
e6dfe8
+    if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
e6dfe8
+        goto cleanup;
e6dfe8
+
e6dfe8
     if (!qemuDomainDiskChangeSupported(disk, orig_disk))
e6dfe8
         goto cleanup;
e6dfe8
 
e6dfe8
@@ -7890,19 +7895,36 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
e6dfe8
                            bool force)
e6dfe8
 {
e6dfe8
     virQEMUDriverPtr driver = dom->conn->privateData;
e6dfe8
+    virDomainDeviceDef oldDev = { .type = dev->type };
e6dfe8
     int ret = -1;
e6dfe8
+    int idx;
e6dfe8
 
e6dfe8
     switch ((virDomainDeviceType) dev->type) {
e6dfe8
     case VIR_DOMAIN_DEVICE_DISK:
e6dfe8
         qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
e6dfe8
         ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force);
e6dfe8
         break;
e6dfe8
+
e6dfe8
     case VIR_DOMAIN_DEVICE_GRAPHICS:
e6dfe8
+        if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics) >= 0)) {
e6dfe8
+            oldDev.data.graphics = vm->def->graphics[idx];
e6dfe8
+            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
e6dfe8
+                return -1;
e6dfe8
+        }
e6dfe8
+
e6dfe8
         ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
e6dfe8
         break;
e6dfe8
+
e6dfe8
     case VIR_DOMAIN_DEVICE_NET:
e6dfe8
+        if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) {
e6dfe8
+            oldDev.data.net = vm->def->nets[idx];
e6dfe8
+            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
e6dfe8
+                return -1;
e6dfe8
+        }
e6dfe8
+
e6dfe8
         ret = qemuDomainChangeNet(driver, vm, dev);
e6dfe8
         break;
e6dfe8
+
e6dfe8
     case VIR_DOMAIN_DEVICE_FS:
e6dfe8
     case VIR_DOMAIN_DEVICE_INPUT:
e6dfe8
     case VIR_DOMAIN_DEVICE_SOUND:
e6dfe8
@@ -8317,6 +8339,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
     virDomainDiskDefPtr newDisk;
e6dfe8
     virDomainGraphicsDefPtr newGraphics;
e6dfe8
     virDomainNetDefPtr net;
e6dfe8
+    virDomainDeviceDef oldDev = { .type = dev->type };
e6dfe8
     int pos;
e6dfe8
 
e6dfe8
     switch ((virDomainDeviceType) dev->type) {
e6dfe8
@@ -8328,6 +8351,10 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
             return -1;
e6dfe8
         }
e6dfe8
 
e6dfe8
+        oldDev.data.disk = vmdef->disks[pos];
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
e6dfe8
+            return -1;
e6dfe8
+
e6dfe8
         virDomainDiskDefFree(vmdef->disks[pos]);
e6dfe8
         vmdef->disks[pos] = newDisk;
e6dfe8
         dev->data.disk = NULL;
e6dfe8
@@ -8343,8 +8370,11 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
             return -1;
e6dfe8
         }
e6dfe8
 
e6dfe8
-        virDomainGraphicsDefFree(vmdef->graphics[pos]);
e6dfe8
+        oldDev.data.graphics = vmdef->graphics[pos];
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
e6dfe8
+            return -1;
e6dfe8
 
e6dfe8
+        virDomainGraphicsDefFree(vmdef->graphics[pos]);
e6dfe8
         vmdef->graphics[pos] = newGraphics;
e6dfe8
         dev->data.graphics = NULL;
e6dfe8
         break;
e6dfe8
@@ -8354,8 +8384,11 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
e6dfe8
         if ((pos = virDomainNetFindIdx(vmdef, net)) < 0)
e6dfe8
             return -1;
e6dfe8
 
e6dfe8
-        virDomainNetDefFree(vmdef->nets[pos]);
e6dfe8
+        oldDev.data.net = vmdef->nets[pos];
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
e6dfe8
+            return -1;
e6dfe8
 
e6dfe8
+        virDomainNetDefFree(vmdef->nets[pos]);
e6dfe8
         vmdef->nets[pos] = net;
e6dfe8
         dev->data.net = NULL;
e6dfe8
         break;
e6dfe8
@@ -8443,7 +8476,7 @@ qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
e6dfe8
         if (!vmdef)
e6dfe8
             goto cleanup;
e6dfe8
 
e6dfe8
-        if (virDomainDefCompatibleDevice(vmdef, dev) < 0)
e6dfe8
+        if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
e6dfe8
             goto cleanup;
e6dfe8
         if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, conn, caps,
e6dfe8
                                                 parse_flags,
e6dfe8
@@ -8452,7 +8485,7 @@ qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
e6dfe8
     }
e6dfe8
 
e6dfe8
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
e6dfe8
-        if (virDomainDefCompatibleDevice(vm->def, dev_copy) < 0)
e6dfe8
+        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
e6dfe8
             goto cleanup;
e6dfe8
 
e6dfe8
         if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, conn, driver)) < 0)
e6dfe8
@@ -8592,9 +8625,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
e6dfe8
         if (!vmdef)
e6dfe8
             goto endjob;
e6dfe8
 
e6dfe8
-        if (virDomainDefCompatibleDevice(vmdef, dev) < 0)
e6dfe8
-            goto endjob;
e6dfe8
-
e6dfe8
+        /* virDomainDefCompatibleDevice call is delayed until we know the
e6dfe8
+         * device we're going to update. */
e6dfe8
         if ((ret = qemuDomainUpdateDeviceConfig(vmdef, dev, caps,
e6dfe8
                                                 parse_flags,
e6dfe8
                                                 driver->xmlopt)) < 0)
e6dfe8
@@ -8602,9 +8634,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
e6dfe8
     }
e6dfe8
 
e6dfe8
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
e6dfe8
-        if (virDomainDefCompatibleDevice(vm->def, dev_copy) < 0)
e6dfe8
-            goto endjob;
e6dfe8
-
e6dfe8
+        /* virDomainDefCompatibleDevice call is delayed until we know the
e6dfe8
+         * device we're going to update. */
e6dfe8
         if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0)
e6dfe8
             goto endjob;
e6dfe8
         /*
e6dfe8
-- 
e6dfe8
2.17.0
e6dfe8