In some cases we want to prepare a @src which is not meant to belong to
a disk and thus does not require us to copy the data. Allow passing in
NULL @disk into qemuDomainPrepareDiskSourceData.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 259cf51e2b..3deb69cb63 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -15095,6 +15095,9 @@ void
 qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk,
                                 virStorageSourcePtr src)
 {
+    if (!disk)
+        return;
+
     /* transfer properties valid only for the top level image */
     if (src == disk->src)
         src->detect_zeroes = disk->detect_zeroes;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 10/18/19 1:10 PM, Peter Krempa wrote:
> In some cases we want to prepare a @src which is not meant to belong to
> a disk and thus does not require us to copy the data. Allow passing in
> NULL @disk into qemuDomainPrepareDiskSourceData.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---

This is worth pushing as a bug fix IMO. Of all callers of
qemuDomainPrepareDiskSourceData, just qemuDomainPrepareDiskSourceLegacy
ensures that disk isn't NULL (and not explicitly, more like a side 
effect of qemuDomainValidateStorageSource being used with disk->src 
there). In all other places there is no guard of disk == NULL.

Or perhaps disk=NULL never happens in the current usage of this function 
and I'm being pendant.  Anyway ....



Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>   src/qemu/qemu_domain.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 259cf51e2b..3deb69cb63 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -15095,6 +15095,9 @@ void
>   qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk,
>                                   virStorageSourcePtr src)
>   {
> +    if (!disk)
> +        return;
> +
>       /* transfer properties valid only for the top level image */
>       if (src == disk->src)
>           src->detect_zeroes = disk->detect_zeroes;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Oct 18, 2019 at 16:08:13 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 10/18/19 1:10 PM, Peter Krempa wrote:
> > In some cases we want to prepare a @src which is not meant to belong to
> > a disk and thus does not require us to copy the data. Allow passing in
> > NULL @disk into qemuDomainPrepareDiskSourceData.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> 
> This is worth pushing as a bug fix IMO. Of all callers of
> qemuDomainPrepareDiskSourceData, just qemuDomainPrepareDiskSourceLegacy
> ensures that disk isn't NULL (and not explicitly, more like a side effect of
> qemuDomainValidateStorageSource being used with disk->src there). In all
> other places there is no guard of disk == NULL.
> 
> Or perhaps disk=NULL never happens in the current usage of this function and
> I'm being pendant.  Anyway ....

It is not a bug fix. Out of the 3 non-test callers, two dereference disk
anyways in other places thus this fix will not help.

In fact it is meant to make the third caller
qemuDomainPrepareStorageSourceBlockdev callable with NULL disk which is
the intended purpose of this patch. That helper is currently used only
in cases where qemuDomainPrepareDiskSourceLegacy would be used and thus
disk must be non-NULL. The use with NULL disk will appear later in this
series.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list