[libvirt] [PATCH 3/3] qemu: domain: Fix backing store terminator for non-backing local files

[John Ferlan]

On 11/24/2017 07:21 AM, Peter Krempa wrote:
> Raw local files do not pass through the backing store detector and thus
> the code did not allocate the required backing store terminator for
> them. Previously the terminating element would be formatted into the XML
> since the default values used for the metadata allowed that. This is a
> regression since a693fdba0111ff which was not detected in the review.

This this is a bug fix to the bug that that patch was attempting to fix?
 I do see it being pointed out as a comment from review that there's a
lot of backingStore removals...  Perhaps better said - the initial patch
was too aggressive but neglected to handle xxxx case?  I'm sure there's
a better way to wordsmith this particular attribution.

> 
> This patch also reverts all the changes in the test files.
> ---
>  src/qemu/qemu_domain.c                                               | 5 +++++

Comparing the files that changed in a693fdba0111ff...

>  .../qemuhotplugtestdomains/qemuhotplug-base-ccw-live+ccw-virtio.xml  | 1 +
>  ...uhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-reverse.xml | 2 ++
>  .../qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml                  | 1 +
>  ...muhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2-explicit.xml | 1 +
>  .../qemuhotplug-base-ccw-live-with-ccw-virtio+ccw-virtio-2.xml       | 1 +

Why does qemuhotplug-base-ccw-live-with-2-ccw-virtio.xml get the
backingStore, but qemuhotplug-base-ccw-live-with-ccw-virtio.xml does not?

I can understand why the "added" hotplug disks have it, but it's unclear
why the "base" file doesn't for the non "with-2" test.


>  .../qemuhotplug-base-live+disk-scsi-wwn+disk-scsi-duplicate-wwn.xml  | 1 +
>  tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-scsi.xml     | 1 +
>  tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-usb.xml      | 1 +
>  tests/qemuhotplugtestdomains/qemuhotplug-base-live+disk-virtio.xml   | 1 +
>  .../qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml    | 1 +
>  11 files changed, 16 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 0cdcb11c37..82671d99c6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6167,6 +6167,11 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
>              goto cleanup;
>          }
> 
> +        /* terminate the chain for such images as the code below would do */
> +        if (!src->backingStore &&
> +            VIR_ALLOC(src->backingStore) < 0)

Should be able to fit on one line

Reviewed-by: John Ferlan <jferlan at redhat.com>

I'm OK w/ this and patch 2 in during freeze - the rules are a bit grey
when it comes to fixing problems without bz's though...