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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux