Re: [libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain

[John Ferlan]

On 10/20/2017 09:47 AM, Peter Krempa wrote:
> When a user provides the backing chain, we will not need to re-detect
> all the backing stores again, but should move to the end of the user
> specified chain. Additionally if a user provides a full terminated chain
> we should not attempt any further detection.
> ---
>  src/qemu/qemu_domain.c | 48 +++-
>  1 file changed, 39 insertions(+), 9 deletions(-)
> 

Looks like some magic combination of keys caused an premature send.

In any case, IDC if you change the noted disk->src to src, but figured
I'd ask anyway...

Reviewed-by: John Ferlan 


> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3560cdd29..5973474ca 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
>   bool report_broken)
>  {
>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> -int ret = 0;
> +virStorageSourcePtr src = disk->src;
> +int ret = -1;
>  uid_t uid;
>  gid_t gid;
> 
> -if (virStorageSourceIsEmpty(disk->src))
> +if (virStorageSourceIsEmpty(src)) {
> +ret = 0;
>  goto cleanup;
> +}
> 
>  if (virStorageSourceHasBacking(disk->src)) {
> -if (force_probe)
> -virStorageSourceBackingStoreClear(disk->src);
> -else
> -goto cleanup;
> +if (force_probe) {
> +virStorageSourceBackingStoreClear(src);
> +} else {
> +/* skip to the end of the chain */
> +while (virStorageSourceIsBacking(src)) {
> +if (report_broken &&
> +virStorageFileSupportsAccess(src)) {
> +
> +if (qemuDomainStorageFileInit(driver, vm, src, 
> disk->src) < 0)
> +goto cleanup;
> +
> +if (virStorageFileAccess(src, F_OK) < 0) {
> +virStorageFileReportBrokenChain(errno, src, 
> disk->src);
> +virStorageFileDeinit(src);
> +goto cleanup;
> +}
> +
> +virStorageFileDeinit(src);
> +}
> +src = src->backingStore;
> +}
> +}
>  }
> 
> -qemuDomainGetImageIds(cfg, vm, disk->src, NULL, , );
> +/* We skipped to the end of the chain. Skip detection if there's the
> + * terminator. (An allocated but empty backingStore) */
> +if (src->backingStore) {
> +ret = 0;
> +goto cleanup;
> +}
> +
> +qemuDomainGetImageIds(cfg, vm, src, disk->src, , );
> 
> -if (virStorageFileGetMetadata(disk->src,
> +if (virStorageFileGetMetadata(src,
>uid, gid,
>cfg->allowDiskFormatProbing,
>report_broken) < 0)
> -ret = -1;
> +goto cleanup;

This looks familiar to the recent upstream question...


> +
> +ret = 0;
> 
>   cleanup:
>  virObjectUnref(cfg);
> 

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

Re: [libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain

[John Ferlan]

On 10/20/2017 09:47 AM, Peter Krempa wrote:
> When a user provides the backing chain, we will not need to re-detect
> all the backing stores again, but should move to the end of the user
> specified chain. Additionally if a user provides a full terminated chain
> we should not attempt any further detection.
> ---
>  src/qemu/qemu_domain.c | 48 +++-
>  1 file changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3560cdd29..5973474ca 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
>   bool report_broken)
>  {
>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> -int ret = 0;
> +virStorageSourcePtr src = disk->src;
> +int ret = -1;
>  uid_t uid;
>  gid_t gid;
> 
> -if (virStorageSourceIsEmpty(disk->src))
> +if (virStorageSourceIsEmpty(src)) {
> +ret = 0;
>  goto cleanup;
> +}
> 
>  if (virStorageSourceHasBacking(disk->src)) {

could this one be just @src?

> -if (force_probe)
> -virStorageSourceBackingStoreClear(disk->src);
> -else
> -goto cleanup;
> +if (force_probe) {
> +virStorageSourceBackingStoreClear(src);
> +} else {
> +/* skip to the end of the chain */
> +while (virStorageSourceIsBacking(src)) {
> +if (report_broken &&
> +virStorageFileSupportsAccess(src)) {
> +
> +if (qemuDomainStorageFileInit(driver, vm, src, 
> disk->src) < 0)
> +goto cleanup;
> +
> +if (virStorageFileAccess(src, F_OK) < 0) {
> +virStorageFileReportBrokenChain(errno, src, 
> disk->src);
> +virStorageFileDeinit(src);
> +goto cleanup;
> +}
> +
> +virStorageFileDeinit(src);
> +}
> +src = src->backingStore;
> +}
> +}
>  }
> 
> -qemuDomainGetImageIds(cfg, vm, disk->src, NULL, , );
> +/* We skipped to the end of the chain. Skip detection if there's the
> + * terminator. (An allocated but empty backingStore) */
> +if (src->backingStore) {
> +ret = 0;
> +goto cleanup;
> +}
> +
> +qemuDomainGetImageIds(cfg, vm, src, disk->src, , );
> 
> -if (virStorageFileGetMetadata(disk->src,
> +if (virStorageFileGetMetadata(src,
>uid, gid,
>cfg->allowDiskFormatProbing,
>report_broken) < 0)
> -ret = -1;
> +goto cleanup;

> +
> +ret = 0;
> 
>   cleanup:
>  virObjectUnref(cfg);
> 

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

[libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain

[Peter Krempa]

When a user provides the backing chain, we will not need to re-detect
all the backing stores again, but should move to the end of the user
specified chain. Additionally if a user provides a full terminated chain
we should not attempt any further detection.
---
 src/qemu/qemu_domain.c | 48 +++-
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3560cdd29..5973474ca 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
  bool report_broken)
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-int ret = 0;
+virStorageSourcePtr src = disk->src;
+int ret = -1;
 uid_t uid;
 gid_t gid;

-if (virStorageSourceIsEmpty(disk->src))
+if (virStorageSourceIsEmpty(src)) {
+ret = 0;
 goto cleanup;
+}

 if (virStorageSourceHasBacking(disk->src)) {
-if (force_probe)
-virStorageSourceBackingStoreClear(disk->src);
-else
-goto cleanup;
+if (force_probe) {
+virStorageSourceBackingStoreClear(src);
+} else {
+/* skip to the end of the chain */
+while (virStorageSourceIsBacking(src)) {
+if (report_broken &&
+virStorageFileSupportsAccess(src)) {
+
+if (qemuDomainStorageFileInit(driver, vm, src, disk->src) 
< 0)
+goto cleanup;
+
+if (virStorageFileAccess(src, F_OK) < 0) {
+virStorageFileReportBrokenChain(errno, src, disk->src);
+virStorageFileDeinit(src);
+goto cleanup;
+}
+
+virStorageFileDeinit(src);
+}
+src = src->backingStore;
+}
+}
 }

-qemuDomainGetImageIds(cfg, vm, disk->src, NULL, , );
+/* We skipped to the end of the chain. Skip detection if there's the
+ * terminator. (An allocated but empty backingStore) */
+if (src->backingStore) {
+ret = 0;
+goto cleanup;
+}
+
+qemuDomainGetImageIds(cfg, vm, src, disk->src, , );

-if (virStorageFileGetMetadata(disk->src,
+if (virStorageFileGetMetadata(src,
   uid, gid,
   cfg->allowDiskFormatProbing,
   report_broken) < 0)
-ret = -1;
+goto cleanup;
+
+ret = 0;

  cleanup:
 virObjectUnref(cfg);
-- 
2.14.1

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