Re: [PATCH] docs/formatstorageencryption.html.in: support qcow2 format in luks encryption volume

[Han Han]

The patch summary could be more short:
docs: support qcow2 format in luks encryption volume

On Thu, Dec 24, 2020 at 2:43 PM Meina Li  wrote:

> Signed-off-by: Meina Li 
> ---
>  docs/formatstorageencryption.html.in | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/docs/formatstorageencryption.html.in b/docs/
> formatstorageencryption.html.in
> index ea80a87cfb..7215c307d7 100644
> --- a/docs/formatstorageencryption.html.in
> +++ b/docs/formatstorageencryption.html.in
> @@ -128,7 +128,9 @@
>
>  
>Here is an example specifying use of the luks format
> for
> -  a specific cipher algorithm for volume creation:
> +  a specific cipher algorithm for volume creation.
> +  Since 6.10.0, the target
> format
> +  can also support qcow2 type with luks
> encryption.
>
Since you mentioned the qcow2 format here, it is better to provide a vol
xml example with 

[PATCH] docs/formatstorageencryption.html.in: support qcow2 format in luks encryption volume

[Meina Li]

Signed-off-by: Meina Li 
---
 docs/formatstorageencryption.html.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index ea80a87cfb..7215c307d7 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -128,7 +128,9 @@
 
 
   Here is an example specifying use of the luks format for
-  a specific cipher algorithm for volume creation:
+  a specific cipher algorithm for volume creation.
+  Since 6.10.0, the target format
+  can also support qcow2 type with luks 
encryption.
 
 
 
-- 
2.27.0

Re: RFC PATCH: Issue 90 (Test Clarification)

[Ryan Gahagan]

On Wed, Dec 23, 2020 at 11:45 AM Peter Krempa  wrote:

> For this test,
> you'll be better off adding a hack to fill in the uid/gid values (e.g.
> by checking that they start with a + to be safe.


Our interpretation of what you mean here is to add a check in our parse
method (specifically in virDomainStorageNetworkParseNFS, which is called by
virDomainDIskSourceNetworkParse in domain_conf.c) which looks at the
user/group strings in the XML and checks if their first character is a '+'.
If so, assume that this string is an integer and not a name, then parse it
into the nfs_uid or gid values. This would allow our tests to understand a
property like "user='+2'" without having to call the preparseStorageSource
methods. This functionality is built into virGetUserID, but because we
can't call it we would have to duplicate the code. Is this an acceptable
approach?


> The formatter omits the values if they are default. According to the QMP
> schema they can be missing. The parser thus must cope when the JSON
> object doesn't have the values populated.
>

So will a method like "virJSONValueObjectGetNumberInt" return an error code
(i.e. < 0) if the property is missing? If so, we could use this error code
as an indicator that the property (e.g. user) should be the default value
(e.g. -1).

We also had some questions in regards to other tests. We won't submit it as
an RFC patch because it isn't ready, but if you'd like to see our current
code for reference you can view it on this branch
.

We're failing virschematest for our file disk-network-nfs.xml. The error is:
"Extra element devices in interleave
Element domain failed to validate content"

This seems like there's a problem with our XML not matching the RNG docs.
However, the problem appears to be with our  tag, which was based
on the XML in disk-backing-chains.xml. Why does our file report an error
for this, but disk-backing-chains.xml does not?

Also, both xml2argv and xml2xml tests are failing, even with the
VIR_REGENERATE_OUTPUT environment variable set to 1. The error is:
"libvirt: Domain Config error : missing source information for device vda"
To be completely honest, we don't know what about our XML or schema is
causing this error and so we aren't sure what the fix is. We've based our
XML mostly on disk-network-vxhs.xml and disk-backing-chains.xml, but don't
understand why those pass and ours doesn't.

Re: RFC PATCH: Issue 90 (Test Clarification)

[Peter Krempa]

On Mon, Dec 21, 2020 at 17:10:35 -0600, Ryan Gahagan wrote:
> We were able to get the testing environment running after some
> trial-and-error. The issue was that the version of libtirpc was out of
> date, causing an error in meson.build when the XDR dependency was being
> read. By updating the libtirpc-dev package we were able to start building
> locally. However, we did have some questions about our tests. For now we
> wanted to focus specifically on the qemublocktest.c file.
> 
> We have the function virDomainDiskSourceNetworkParse in
> src/conf/domain_conf.c where XML is parsed into virStorageSource data and
> the function qemuBlockStorageSourceCreateGetStorageProps under
> src/qemu/qemu_block.c where this data is converted into a JSON object. Our
> understanding of the tests/qemublocktest.c file is that we provide an XML
> string example via TEST_JSON_FORMAT_NET and that this string was parsed
> into JSON then formatted back into XML to make sure the process produced
> the same output as the input. One issue is that for some reason our user
> and group strings (i.e. “”) are not getting
> parsed into integers. The nfs_uid and nfs_gid fields are never set and
> therefore result in a 0 every time. We have code to fix this in a method
> under qemuDomainPrepareStorageSourceBlockdev in the file
> src/qemu/qemu_domain.c which looks up the user and group and stores the
> values we expect, but this method isn’t getting called. Where in this
> process have we missed something? Should we move the virGetUserID and
> virGetGroupID method calls to somewhere else?

No. The issue is that the tests shouldn't really be allowed to look up
the UID/GID in the system. For that reason the test code doesn't
actually call qemuDomainPrepareStorageSourceBlockdev. For this test,
you'll be better off adding a hack to fill in the uid/gid values (e.g.
by checking that they start with a + to be safe.

> We were also curious about the methods which parse the JSON values back
> into data. In our case, this is virStorageSourceParseBackingJSONNFS in
> src/util/virstoragefile.c. This method tries to retrieve the user and group
> integers from the NFS JSON object. However, when constructing the JSON
> object in the getStorageProps method we don't actually add those values if
> they're "default" values. In JSON terms this means that the integers are
> undefined, but how does C interpret it? Does the retrieve method return 0
> or simply fail?

The formatter omits the values if they are default. According to the QMP
schema they can be missing. The parser thus must cope when the JSON
object doesn't have the values populated.