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.d...@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.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to