On 02/22/2018 09:21 AM, Jiri Denemark wrote:
> Commit v3.7.0-14-gc57f3fd2f8 prevented adding a
> element to an inactive domain with global 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
> ---
> 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)
> + ) < 0)
> return -1;
> }
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list