conf: Forbid device alias change on device-update

https://bugzilla.redhat.com/show_bug.cgi?id=1585108

When updating a live device users might pass different alias than
the one the device has. Currently, this is silently ignored which
goes against our behaviour for other parts of the device where we
explicitly allow only certain changes and error out loudly on
anything else.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93cfca351c95ff5d42d34d0db9d353394586f586..b8b53450fa1689bec6fcb3693b24f4c5ba76feab 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28206,7 +28206,8 @@ int
 virDomainDefCompatibleDevice(virDomainDefPtr def,
                              virDomainDeviceDefPtr dev,
                              virDomainDeviceDefPtr oldDev,
-                             virDomainDeviceAction action ATTRIBUTE_UNUSED)
+                             virDomainDeviceAction action,
+                             bool live)
 {
     virDomainCompatibleDeviceData data = {
         .newInfo = virDomainDeviceGetInfo(dev),
@@ -28216,6 +28217,16 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
     if (oldDev)
         data.oldInfo = virDomainDeviceGetInfo(oldDev);
 
+    if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
+        live &&
+        ((!!data.newInfo != !!data.oldInfo) ||
+         (data.newInfo && data.oldInfo &&
+          STRNEQ_NULLABLE(data.newInfo->alias, data.oldInfo->alias)))) {
+        virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+                       _("changing device alias is not allowed"));
+        return -1;
+    }
+
     if (!virDomainDefHasUSB(def) &&
         def->os.type != VIR_DOMAIN_OSTYPE_EXE &&
         virDomainDeviceIsUSB(dev)) {

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f33405e09770843467526a65c80232e91c710868..71437dc4851ab4bd487ea6fc64d821ab860d43b3 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3107,7 +3107,8 @@ typedef enum {
 int virDomainDefCompatibleDevice(virDomainDefPtr def,
                                  virDomainDeviceDefPtr dev,
                                  virDomainDeviceDefPtr oldDev,
-                                 virDomainDeviceAction action);
+                                 virDomainDeviceAction action,
+                                 bool live);
 
 void virDomainRNGDefFree(virDomainRNGDefPtr def);
 

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ef225999d8183efc784c5e4a1bb91fed8c84dc59..f9794e0655c3d6f2b61610841250a53462404490 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3522,7 +3522,8 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         oldDev.data.net = vmdef->nets[idx];
         if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
-                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                         false) < 0)
             return -1;
 
         virDomainNetDefFree(vmdef->nets[idx]);
@@ -4759,7 +4760,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
             goto endjob;
 
         if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
-                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+                                         false) < 0)
             goto endjob;
 
         if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
@@ -4768,7 +4770,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
-                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+                                         true) < 0)
             goto endjob;
 
         if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) < 0)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f8a662f7475d18f67cb9b4ed1026062701e8be05..fee44812c11dbbb26c6abcd7b7ce27b15de05c39 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8643,13 +8643,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
         return false;
     }
 
-    if (disk->info.alias &&
-        STRNEQ_NULLABLE(disk->info.alias, orig_disk->info.alias)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("cannot modify field '%s' of the disk"),
-                       "alias");
-        return false;
-    }
+    /* device alias is checked already in virDomainDefCompatibleDevice */
 
     CHECK_EQ(info.bootIndex, "boot order", true);
     CHECK_EQ(rawio, "rawio", true);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0e3ac8c741cffec8b8fefe7008593996234dc7c8..825b2b27e65c302bd775b3eaef3a1c31af5e1856 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7861,7 +7861,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
 
     oldDev.data.disk = orig_disk;
     if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
-                                     VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                     VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                     true) < 0)
         goto cleanup;
 
     if (!qemuDomainDiskChangeSupported(disk, orig_disk))
@@ -7920,7 +7921,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
         if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) >= 0) {
             oldDev.data.graphics = vm->def->graphics[idx];
             if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
-                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                             true) < 0)
                 return -1;
         }
 
@@ -7931,7 +7933,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
         if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) {
             oldDev.data.net = vm->def->nets[idx];
             if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
-                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                             true) < 0)
                 return -1;
         }
 
@@ -8385,7 +8388,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         oldDev.data.disk = vmdef->disks[pos];
         if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
-                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                         false) < 0)
             return -1;
 
         virDomainDiskDefFree(vmdef->disks[pos]);
@@ -8405,7 +8409,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         oldDev.data.graphics = vmdef->graphics[pos];
         if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
-                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                         false) < 0)
             return -1;
 
         virDomainGraphicsDefFree(vmdef->graphics[pos]);
@@ -8420,7 +8425,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         oldDev.data.net = vmdef->nets[pos];
         if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
-                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+                                         false) < 0)
             return -1;
 
         virDomainNetDefFree(vmdef->nets[pos]);
@@ -8512,7 +8518,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
             goto cleanup;
 
         if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
-                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+                                         false) < 0)
             goto cleanup;
         if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
                                                 parse_flags,
@@ -8522,7 +8529,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
-                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+                                         true) < 0)
             goto cleanup;
 
         if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 965b051fd71bc019b0724b30289dc6b83d761177..fcd8eb0ffad1f72352c9f925dc3ece7c5278f306 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3200,11 +3200,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
     if (!newdev->info.alias &&
         VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0)
         goto cleanup;
-    if (STRNEQ_NULLABLE(olddev->info.alias, newdev->info.alias)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("cannot modify network device alias"));
-        goto cleanup;
-    }
+
+    /* device alias is checked already in virDomainDefCompatibleDevice */
+
     if (olddev->info.rombar != newdev->info.rombar) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network device rom bar setting"));