Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]

On 05/20/2014 04:01 PM, Tomoki Sekiyama wrote:
> The patch below is for the command to get filesystems list.
> 
> ===
> From: Tomoki Sekiyama <tomoki.sekiyama@hds.com>

You'll want to resend it as a series of patches as a top-level thread;
not everyone notices a patch buried as a reply.

> 
> qga: Add guest-get-fs-info command
> 
> Add command to get mounted filesystems information in the guest.
> The returned value contains a list of mountpoint paths and
> corresponding disks info such as disk bus type, drive address,
> and the disk controllers' PCI addresses, so that management layer
> such as libvirt can resolve the disk backends.
> In Linux guest, the disk information is resolved from sysfs.
> 
> guest-get-fs-info command will return the following result:
> 
>     {"return":
>      [{"name":"dm-1",
>        "mountpoint":"/mnt/test",
>        "disk":[
>         {"bus-type":"scsi","bus":0,"unit":1,"target":0,
>          "pci-controller":{"bus":0,"slot":10,"domain":0,"function":0}},
>         {"bus-type":"ide","bus":0,"unit":0,"target":0,
>          "pci-controller":{"bus":0,"slot":1,"domain":0,"function":1}}],
>        "type":"xfs"},

Definitely looks like a useful set of information.  I have not reviewed
the patch closely, but it looks like you have a good API for Linux.  I'm
a bit worried that it might be hard to translate this API into non-Linux
guests, but it still seems generic enough; and the argument also holds
that if this command is not implemented, the guest does not support
partial freezing.

> +++ b/qga/qapi-schema.json
> @@ -646,3 +646,80 @@
>  { 'command': 'guest-set-vcpus',
>    'data':    {'vcpus': ['GuestLogicalProcessor'] },

> +
> +##
> +# @GuestFilesystemInfo
> +#
> +# @name: disk name
> +# @mountpoint: mount point path
> +# @type: file system type string
> +# @disk: an array of disk hardware informations that the volume lies on

s/informations/information/

> +#
> +# Since: 2.1
> +##
> +{ 'type': 'GuestFilesystemInfo',
> +  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
> +           '*disk': ['GuestDiskAddress']} }

Why is the array optional? Is it something where an empty array has
different meaning than omitting the array altogether?

> +
> +##
> +# @guest-get-fs-info:
> +#
> +# Returns: The list of filesystems information mounted in the guest.
> +#          The returned mountpoints may be specified to @guest-fsfreeze-freeze.
> +#          Network filesystems (such as CIFS and NFS) are not listed.
> +#
> +# Since: 2.1
> +##
> +{ 'command': 'guest-get-fs-info',
> +  'returns': ['GuestFilesystemInfo'] }

Should this command support an optional input argument '*name':'str'
that filters the results to a one-element array about that one named
mountpoint?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]