From 4ad54a417a1bbe10aeffcce783f4381d8d43f799 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 12 Jun 2018 16:05:10 +0200 Subject: [PATCH] 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 Reviewed-by: John Ferlan --- src/conf/domain_conf.c | 13 ++++++++++++- src/conf/domain_conf.h | 3 ++- src/lxc/lxc_driver.c | 9 ++++++--- src/qemu/qemu_domain.c | 8 +------- src/qemu/qemu_driver.c | 24 ++++++++++++++++-------- src/qemu/qemu_hotplug.c | 8 +++----- 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 93cfca351c..b8b53450fa 100644 --- 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 f33405e097..71437dc485 100644 --- 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 ef225999d8..f9794e0655 100644 --- 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 f8a662f747..fee44812c1 100644 --- 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 0e3ac8c741..825b2b27e6 100644 --- 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 965b051fd7..fcd8eb0ffa 100644 --- 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")); -- GitLab