[PATCH 0/2] virsh: Fix bug in XML generator of cmdBlockcopy
Peter Krempa (2): virsh: cmdBlockcopy: Add '--print-xml' flag virsh: cmdBlockcopy: Fix generator of block copy disk XML docs/manpages/virsh.rst | 4 +++- tools/virsh-domain.c | 24 +++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) -- 2.35.1
Useful for knowing how to construct the XML and debugging. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 4 +++- tools/virsh-domain.c | 13 ++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e8568559fa..e73e590754 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1232,7 +1232,7 @@ blockcopy [--shallow] [--reuse-external] [bandwidth] [--wait [--async] [--verbose]] [{--pivot | --finish}] [--timeout seconds] [granularity] [buf-size] [--bytes] - [--transient-job] [--synchronous-writes] + [--transient-job] [--synchronous-writes] [--print-xml] Copy a disk backing image chain to a destination. Either *dest* as the destination file name, or *--xml* with the name of an XML file containing @@ -1297,6 +1297,8 @@ to be propagated both to the original image and to the destination of the copy so that it's guaranteed that the job converges if the destination storage is slower. This may impact performance of writes while the blockjob is running. +If *--print-xml* is specified, then the XML used to start the block copy job +is printed instead of starting the job. blockjob -------- diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index df8df9c2f3..452e518156 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2332,6 +2332,10 @@ static const vshCmdOptDef opts_blockcopy[] = { .type = VSH_OT_BOOL, .help = N_("the copy job forces guest writes to be synchronously written to the destination") }, + {.name = "print-xml", + .type = VSH_OT_BOOL, + .help = N_("print the XML used to start the copy job instead of starting the job") + }, {.name = NULL} }; @@ -2360,6 +2364,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) int abort_flags = 0; const char *xml = NULL; char *xmlstr = NULL; + bool print_xml = vshCommandOptBool(cmd, "print-xml"); virTypedParameterPtr params = NULL; virshBlockJobWaitData *bjWait = NULL; int nparams = 0; @@ -2437,7 +2442,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) } if (granularity || buf_size || (format && STRNEQ(format, "raw")) || xml || - transientjob || syncWrites) { + transientjob || syncWrites || print_xml) { /* New API */ if (bandwidth || granularity || buf_size) { params = g_new0(virTypedParameter, 3); @@ -2492,6 +2497,12 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) xmlstr = virBufferContentAndReset(&buf); } + if (print_xml) { + vshPrint(ctl, "%s", xmlstr); + ret = true; + goto cleanup; + } + if (virDomainBlockCopy(dom, path, xmlstr, params, nparams, flags) < 0) goto cleanup; } else { -- 2.35.1
In a recent commit I've attempted to rewrite the XML generator to use virXMLFormatElement instead of manual steps. Unfortunately the commit had multiple problems resulting in a garbled XML: 1) in certain cases the wrong buffer was used resulting in misplaced snippets 2) the child element buffer was improperly set up so sub-elements were not indented This resulted in following XML being generated: $ virsh blockcopy cd vda /tmp/test.copy --raw --print-xml type='file''/tmp/test.copy'/> <driver type='raw'/> <disk> <source file=</disk> To fix this we'll generate the '<source>' element in one go and use the proper buffer for it and other places. Fixes: 1cd95f858ab83f2baab0e35070d00766bb06ce3a Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2078274 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 452e518156..ba492e807e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2481,18 +2481,17 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) if (!xmlstr) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(&buf); if (blockdev) { virBufferAddLit(&attrBuf, " type='block'"); - virBufferAddLit(&childBuf, "<source dev="); + virBufferEscapeString(&childBuf, "<source dev='%s'/>\n", dest); } else { - virBufferAddLit(&buf, " type='file'"); - virBufferAddLit(&childBuf, "<source file="); + virBufferAddLit(&attrBuf, " type='file'"); + virBufferEscapeString(&childBuf, "<source file='%s'/>\n", dest); } - virBufferEscapeString(&buf, "'%s'/>\n", dest); - virBufferEscapeString(&buf, "<driver type='%s'/>\n", format); + virBufferEscapeString(&childBuf, "<driver type='%s'/>\n", format); virXMLFormatElement(&buf, "disk", &attrBuf, &childBuf); xmlstr = virBufferContentAndReset(&buf); } -- 2.35.1
On 4/25/22 10:37, Peter Krempa wrote:
Peter Krempa (2): virsh: cmdBlockcopy: Add '--print-xml' flag virsh: cmdBlockcopy: Fix generator of block copy disk XML
docs/manpages/virsh.rst | 4 +++- tools/virsh-domain.c | 24 +++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa