[PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Or Ozeri posted 5 patches 2 months ago

[PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Posted by Or Ozeri 2 months ago
The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.

Signed-off-by: Or Ozeri <oro@il.ibm.com>
---
 src/qemu/qemu_domain.c | 2 +-
 src/qemu/qemu_domain.h | 4 ++++
 tests/qemublocktest.c  | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 25b7f03204..472ff670b1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5415,7 +5415,7 @@ qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
                                  virQEMUCaps *qemuCaps,
                                  unsigned int parseFlags)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cb1cd968d5..9a784501a0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -899,6 +899,10 @@ int qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
                                     virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+                                     virQEMUCaps *qemuCaps,
+                                     unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
                              const char *domainChannelTargetDir)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 4af8862c5b..617e1b8ae1 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -279,6 +279,9 @@ testQemuDiskXMLToProps(const void *opaque)
                                        VIR_DOMAIN_DEF_PARSE_STATUS)))
         return -1;
 
+    if (qemuDomainDeviceDiskDefPostParse(disk, data->qemuCaps, 0) < 0)
+        return -1;
+
     if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
         return -1;
 
-- 
2.25.1

Re: [PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Posted by Peter Krempa 2 months ago
On Tue, Oct 05, 2021 at 09:41:12 -0500, Or Ozeri wrote:
> The post parse callback is part of the real (non-test) processing flow.
> This commit adds it (for disks) to the qemublocktest flow as well.

Could you please elaborate why this is needed? Specifically
qemublocktest takes a few liberties from the "real" code flow since we
need to fake a lot of stuff. E.g. see testQemuDiskXMLToJSONFakeSecrets.

Specifically I didn't see anything in your patches [1] which would add
anything to the post parse callback.


[1] Well after my crude rebase of the series. What you've posted didn't
apply neither on master nor on the last release. I had to go one version
back and it had conflicts which I didn't spend much time thinking about.

RE: [PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Posted by Or Ozeri 2 months ago

                
            

Re: [PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Posted by Peter Krempa 2 months ago
On Wed, Oct 06, 2021 at 09:55:10 +0000, Or Ozeri wrote:
>    My commit which adds the encryption engine configuration is default to
>    "default".
>    Only in the post-parse callback in the qemu driver, I switch it from
>    "default" to "qemu".
>    Thus, if you don't call the post-parse on this test, then tests which use
>    luks format fail.
>    Specifically:
>    network-qcow2-backing-chain-encryption_auth
>    file-qcow2-backing-chain-encryption
>    file-raw-luks

Yeah, that is okay. As noted in another reply please mention it in the
commit message, because otherwise it seems a bit misplaced. Specifically
exporting just a bit of the post-parse callbacks is questionable but
okay in this case.

And please post a rebased version as noted elsewhere.

Re: [PATCH v2 1/5] qemu: add disk post parse to qemublocktest

Posted by Peter Krempa 2 months ago
On Wed, Oct 06, 2021 at 08:37:16 +0200, Peter Krempa wrote:
> On Tue, Oct 05, 2021 at 09:41:12 -0500, Or Ozeri wrote:
> > The post parse callback is part of the real (non-test) processing flow.
> > This commit adds it (for disks) to the qemublocktest flow as well.
> 
> Could you please elaborate why this is needed? Specifically
> qemublocktest takes a few liberties from the "real" code flow since we
> need to fake a lot of stuff. E.g. see testQemuDiskXMLToJSONFakeSecrets.
> 
> Specifically I didn't see anything in your patches [1] which would add
> anything to the post parse callback.
> 
> 
> [1] Well after my crude rebase of the series. What you've posted didn't
> apply neither on master nor on the last release. I had to go one version
> back and it had conflicts which I didn't spend much time thinking about.

Okay I messed up the rebase and lost the hunk from patch 3.

As of such, please add a note into the commit message that it's needed
to fill in the encryption format default since we are processing old
XMLs.

Additionally please repost the patches rebased to current git master, so
that I don't have to second-guess.