On 06/12/2018 11:04 AM, Michal Privoznik wrote: > 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@xxxxxxxxxx> > --- > src/conf/domain_conf.c | 12 +++++++++++- > src/conf/domain_conf.h | 3 ++- > src/lxc/lxc_driver.c | 6 +++--- > src/qemu/qemu_driver.c | 16 ++++++++-------- > 4 files changed, 24 insertions(+), 13 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 85f07af46e..91cac75c0a 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -28195,7 +28195,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, > int > virDomainDefCompatibleDevice(virDomainDefPtr def, > virDomainDeviceDefPtr dev, > - virDomainDeviceDefPtr oldDev) > + virDomainDeviceDefPtr oldDev, > + bool live) > { > virDomainCompatibleDeviceData data = { > .newInfo = virDomainDeviceGetInfo(dev), > @@ -28205,6 +28206,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, > if (oldDev) > data.oldInfo = virDomainDeviceGetInfo(oldDev); > > + if (live && > + ((!!data.newInfo != !!data.oldInfo) || > + (data.newInfo && data.oldInfo && > + STRNEQ(data.newInfo->alias, data.oldInfo->alias)))) { > + virReportError(VIR_ERR_OPERATION_DENIED, "%s", > + _("changing device alias is not allowed")); > + return -1; > + } > + Does this work for attach-device from the bz? I just tried it and it doesn't - probably because oldInfo would be NULL for attach. Additionally, if the update/detach doesn't include an alias, then would this fail too? By comparison qemuDomainDiskChangeSupported only cares that the updated disk definition has an alias before making check and of course that would now be duplicitous to this. Then there's qemuDomainChangeNet which at first I didn't understand why it wasn't throwing up since it does check the alias. Then I noticed something from the case, for the updated interface XML - if you include the <address> from the live guest, but change the "<alias>", then you get the error. However, if you remove the <address> from the update XML things then succeed. That's because of this gem in the code: if (!virDomainDeviceAddressIsValid(&newdev->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) { goto cleanup; } where virDomainDeviceInfoCopy essentially *overwrites* the provided/changed newdev->info.alias. So I think if you fix this part of the code, then you don't have the need for the rest of this patch. Yes, that does possibly leave the graphics devices as not checking alias updates, but that would seemingly be fixable separately if it's considered a problem or not checked as I didn't follow all the graphics update code. John NB: Another inconsistency in the bz is that the add interface added uses target dev='vnet0', but the live guest shows target dev='vnet12'. Then the updated xml doesn't change target dev='vnet0', but does change the alias and it works? For me it failed with: error: Operation not supported: cannot modify network device tap name until I changed it to match the live xml target dev... [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
