[libvirt] [PATCH 0/2] Fix some storage vol formatting issues (code and doc)

While reviewing another set of changes I tripped across this. John Ferlan (2): conf: Format the "nocow" field. docs: Adjust pool/vol target output docs/formatstorage.html.in | 16 ++++++++-------- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 4 files changed, 12 insertions(+), 8 deletions(-) -- 2.7.4

Commit id 'a9fd30e63' added a '<nocow/>' field to be parsed, but didn't Format the field on output. This patch fixes that and adds the vol-qcow2-nocow test to/for storagevolxml2xml Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 3 files changed, 4 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7e7bb72..dc7cc25 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1590,6 +1590,8 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, return -1; virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat); + if (def->nocow) + virBufferAddLit(buf, "<nocow/>\n"); if (def->features) { size_t i; diff --git a/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml b/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml index 31dc578..a67f807 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml @@ -17,6 +17,7 @@ <encryption format='qcow'> <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> </encryption> + <nocow/> </target> <backingStore> <path>/dev/null</path> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index db82bea..5a0e096 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -115,6 +115,7 @@ mymain(void) DO_TEST("pool-gluster", "vol-gluster-dir-neg-uid"); DO_TEST_FULL("pool-dir", "vol-qcow2-nocapacity", VIR_VOL_XML_PARSE_NO_CAPACITY); + DO_TEST("pool-dir", "vol-qcow2-nocow"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.7.4

On 07.12.2016 16:31, John Ferlan wrote:
Commit id 'a9fd30e63' added a '<nocow/>' field to be parsed, but didn't Format the field on output. This patch fixes that and adds the vol-qcow2-nocow test to/for storagevolxml2xml
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 3 files changed, 4 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7e7bb72..dc7cc25 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1590,6 +1590,8 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, return -1;
virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat); + if (def->nocow) + virBufferAddLit(buf, "<nocow/>\n");
I wonder whether we should detect this flag over a volume and set it so that vol-dumpxml reports the flag even after libvirtd restart. Michal

On Fri, Dec 09, 2016 at 03:38:09PM +0100, Michal Privoznik wrote:
On 07.12.2016 16:31, John Ferlan wrote:
Commit id 'a9fd30e63' added a '<nocow/>' field to be parsed, but didn't Format the field on output. This patch fixes that and adds the vol-qcow2-nocow test to/for storagevolxml2xml
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 3 files changed, 4 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7e7bb72..dc7cc25 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1590,6 +1590,8 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, return -1;
virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat); + if (def->nocow) + virBufferAddLit(buf, "<nocow/>\n");
I wonder whether we should detect this flag over a volume and set it so that vol-dumpxml reports the flag even after libvirtd restart.
Well, I don't think we output things we cannot detect (like "prealloc_metadata") which we accept on input. So if we output it, we need to be able to detect it. If we only accept it on input, then we should not output it (because we cannot guarantee reproducibility after daemon restart). Martin
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 12/09/2016 09:52 AM, Martin Kletzander wrote:
On Fri, Dec 09, 2016 at 03:38:09PM +0100, Michal Privoznik wrote:
On 07.12.2016 16:31, John Ferlan wrote:
Commit id 'a9fd30e63' added a '<nocow/>' field to be parsed, but didn't Format the field on output. This patch fixes that and adds the vol-qcow2-nocow test to/for storagevolxml2xml
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 3 files changed, 4 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7e7bb72..dc7cc25 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1590,6 +1590,8 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, return -1;
virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat); + if (def->nocow) + virBufferAddLit(buf, "<nocow/>\n");
I wonder whether we should detect this flag over a volume and set it so that vol-dumpxml reports the flag even after libvirtd restart.
Well, I don't think we output things we cannot detect (like "prealloc_metadata") which we accept on input. So if we output it, we need to be able to detect it. If we only accept it on input, then we should not output it (because we cannot guarantee reproducibility after daemon restart).
Martin
Michal
Hmmm. detection after restart would only be possible if we can probe the file and so far it only matters for btrfs... Ummm.. I'll just drop this then John

Looks like the <timestamps> and <encryption> were put in the wrong place... They're not <pool> elements, rather they are <volume> elements Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatstorage.html.in | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index ed1f0d1..f1be511 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -396,14 +396,6 @@ <mode>0744</mode> <label>virt_image_t</label> </permissions> - <timestamps> - <atime>1341933637.273190990</atime> - <mtime>1341930622.047245868</mtime> - <ctime>1341930622.047245868</ctime> - </timestamps> - <encryption type='...'> - ... - </encryption> </target> </pool></pre> @@ -585,6 +577,14 @@ <mode>0744</mode> <label>virt_image_t</label> </permissions> + <timestamps> + <atime>1341933637.273190990</atime> + <mtime>1341930622.047245868</mtime> + <ctime>1341930622.047245868</ctime> + </timestamps> + <encryption type='...'> + ... + </encryption> <compat>1.1</compat> <nocow/> <features> -- 2.7.4

On 07.12.2016 16:31, John Ferlan wrote:
While reviewing another set of changes I tripped across this.
John Ferlan (2): conf: Format the "nocow" field. docs: Adjust pool/vol target output
docs/formatstorage.html.in | 16 ++++++++-------- src/conf/storage_conf.c | 2 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 1 + tests/storagevolxml2xmltest.c | 1 + 4 files changed, 12 insertions(+), 8 deletions(-)
ACK series. Michal
participants (3)
-
John Ferlan
-
Martin Kletzander
-
Michal Privoznik