[libvirt] [PATCH v2] libvirt: support block device storage type in virshParseSnapshotDiskspec

[Peter Krempa]

On Mon, Jul 08, 2019 at 17:46:34 +0800, Liu Dayu wrote:
> virsh snapshot-create-as supports 'file' storage type in --diskspec by default.
> But it doesn't support 'block' storage type in the virshParseSnapshotDiskspec().
> So if a snapshot on a block device (e.g. LV) was created, the type of
> current running storage source in dumpxml is inconsistent with the actual
> backend storage source. It will check file-system type mismatch failed
> and return an error message of 'Migration without shared storage is unsafe'
> when VM performs a live migration after this snapshot.
> 
> Considering virsh has to be able to work remotely that recognizing a block device
> by prefix /dev/ or by stat() may be not suitable, so adding a "stype" field
> for the --diskspec string which will be either "file" or "block".
> e.g. --diskspec vda,snapshot=external,driver=qcow2,stype=block,file=/dev/xxx.
> 
> Signed-off-by: Liu Dayu <liu.dayu at zte.com.cn>
> ---
> Patch v1:
> https://www.redhat.com/archives/libvir-list/2019-June/msg01248.html
> 
> Changes in v2:
>  - Adding a "stype" field for the --diskspec string which will indicate a "file" or "block" storage type.
> ---
>  tools/virsh-snapshot.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)

[...]

> @@ -275,13 +279,26 @@ virshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str)
>      virBufferEscapeString(buf, "<disk name='%s'", name);
>      if (snapshot)
>          virBufferAsprintf(buf, " snapshot='%s'", snapshot);
> +    if (stype) {
> +        if (STREQ(stype, "block")) {
> +            isFile = false;
> +        } else if (STRNEQ(stype, "file")) {
> +            vshError(ctl, _("Unknown storage type: '%s'"), stype);
> +            goto cleanup;
> +        }
> +        virBufferAsprintf(buf, " type='%s'", stype);
> +    } 

There is a (invisible) trailing whitespace. I'll fix it.

>      if (driver || file) {

[...]

> @@ -351,7 +368,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
>      },
>      {.name = "diskspec",
>       .type = VSH_OT_ARGV,
> -     .help = N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")
> +     .help = N_("disk attributes: disk[,snapshot=type][,driver=type][,stype=type][,file=name]")
>      },

This patch should also add docs to tools/virsh.pod which is used to
generate the man page for virsh. I'll add some docs before pushing.

Thanks for addressing my comments I'll push this one shortly with the
aforementioned changes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190709/76458115/attachment-0001.sig>