On 02/22/2018 09:21 AM, Jiri Denemark wrote: > Commit v3.7.0-14-gc57f3fd2f8 prevented adding a <boot order='x'/> > element to an inactive domain with global <boot dev='...'/> element. > However, as a result of that change updating any device with boot order > would fail with 'boot order X is already used by another device', where > "another device" is in fact the device which is being updated. > > To fix this we have to ignore the device which we're about to update > when checking for boot order conflicts. > > https://bugzilla.redhat.com/show_bug.cgi?id=1546971 > > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx> > --- > src/conf/domain_conf.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > While chasing something else - I've tripped across this change... > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index c71c28e8d2..d96b012b98 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -27381,18 +27381,30 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev) > return false; > } > > + > +typedef struct _virDomainCompatibleDeviceData virDomainCompatibleDeviceData; > +typedef virDomainCompatibleDeviceData *virDomainCompatibleDeviceDataPtr; > +struct _virDomainCompatibleDeviceData { > + virDomainDeviceInfoPtr newInfo; > + virDomainDeviceInfoPtr oldInfo; > +}; > + > static int > virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, > virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, > virDomainDeviceInfoPtr info, > void *opaque) > { > - virDomainDeviceInfoPtr newinfo = opaque; > + virDomainCompatibleDeviceDataPtr data = opaque; > > - if (info->bootIndex == newinfo->bootIndex) { > + /* Ignore the device we're about to update */ > + if (data->oldInfo == info) > + return 0; > + > + if (info->bootIndex == data->newInfo->bootIndex) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > _("boot order %u is already used by another device"), > - newinfo->bootIndex); > + data->newInfo->bootIndex); > return -1; > } > return 0; > @@ -27401,9 +27413,12 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, > int > virDomainDefCompatibleDevice(virDomainDefPtr def, > virDomainDeviceDefPtr dev, > - virDomainDeviceDefPtr oldDev ATTRIBUTE_UNUSED) > + virDomainDeviceDefPtr oldDev) > { > - virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); > + virDomainCompatibleDeviceData data = { > + .newInfo = virDomainDeviceGetInfo(dev), > + .oldInfo = virDomainDeviceGetInfo(oldDev), > + }; I believe this has broken the ability to attach or update a CDROM for both qemu and lxc as virDomainDefCompatibleDevice is called with a NULL 3rd parameter leading to virDomainDeviceGetInfo(NULL) being called resulting in a fairly immediate coredump. If I modify things to have .oldInfo = NULL, and then fill it in only if @oldDev, that resolves the problem. Whether that's the right fix not 100% sure (I'm still trying to mentally dig out from a week away from work). John > > if (!virDomainDefHasUSB(def) && > def->os.type != VIR_DOMAIN_OSTYPE_EXE && > @@ -27414,7 +27429,7 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, > return -1; > } > > - if (info && info->bootIndex > 0) { > + if (data.newInfo && data.newInfo->bootIndex > 0) { > if (def->os.nBootDevs > 0) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("per-device boot elements cannot be used" > @@ -27423,7 +27438,7 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, > } > if (virDomainDeviceInfoIterate(def, > virDomainDeviceInfoCheckBootIndex, > - info) < 0) > + &data) < 0) > return -1; > } > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list