Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
Hi Eric, thank you for the comments.
On 5/20/14 18:46 , "Eric Blake" wrote:
>On 05/20/2014 04:01 PM, Tomoki Sekiyama wrote:
>> The patch below is for the command to get filesystems list.
>>
>> ===
>> From: Tomoki Sekiyama
>
>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.
OK. I will resend as a series (with a fix below).
>>
>> 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/
Will fix it.
>> +#
>> +# 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?
Not actually. Currently it omits the array when the disk type is not
supported (non-PCI device like floppy disks or SD cards). But it can
be replaced with an empty array if preferable.
>>+
>> +##
>> +# @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?
Hmm, I don't have specific use-cases for the name argument.
As other commands like 'guest-network-get-interfaces' don't have a such
filter, I don't have a reason to give a filter for this command.
Anyway, the caller can easily filter the result by him/herself.
Regards,
Tomoki Sekiyama
Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
On 05/20/2014 04:01 PM, Tomoki Sekiyama wrote:
> The patch below is for the command to get filesystems list.
>
> ===
> From: Tomoki Sekiyama
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
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
> On 04/28/2014 09:25 AM, Tomoki Sekiyama wrote:
>> When an array of mount point paths is specified as 'mountpoints' argument
>> of guest-fsfreeze-freeze, qemu-ga with this patch will only freeze the file
>> systems mounted on specified paths in Linux.
>
> How does the management application learn the set of valid mountpoints
> arguments it can pass to this command? Shouldn't there be a query
> counterpart that asks the guest to return the full list of mountpoints
> that it can support freezing? In the case of guests that are
> all-or-none, like Windows, the query command would return an empty list
> to make it obvious there is no ability to freeze just a subset.
>
> In returning a list of mountpoint names, it might also be nice to
> correlate which names map to which devices (it can be a many:many
> mapping, thanks to things like RAID setups in the guest).
I agree.
The patch below is for the command to get filesystems list.
===
From: Tomoki Sekiyama
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.
For example, when `lsblk' result is:
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sdb 8:16 01G 0 disk
`-sdb1 8:17 0 1024M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
sdc 8:32 01G 0 disk
`-sdc1 8:33 0 512M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
vda252:00 25G 0 disk
`-vda1 252:10 25G 0 part /
where sdb is a SCSI disk with PCI controller :00:0a.0 and ID=1,
sdc is an IDE disk with PCI controller :00:01.1, and
vda is a virtio disk with PCI device :00:06.0,
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"},
{"name":"vda1", "mountpoint":"/",
"disk":[
{"bus-type":"virtio","bus":0,"unit":0,"target":0,
"pci-controller":{"bus":0,"slot":6,"domain":0,"function":0}}],
"type":"ext4"}]}
Signed-off-by: Tomoki Sekiyama
---
qga/commands-posix.c | 420 ++-
qga/qapi-schema.json | 77 ++
2 files changed, 496 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 771f00c..212913a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -575,6 +576,7 @@ static void guest_file_init(void)
typedef struct FsMount {
char *dirname;
char *devtype;
+unsigned int devmajor, devminor;
QTAILQ_ENTRY(FsMount) next;
} FsMount;
@@ -596,15 +598,40 @@ static void free_fs_mount_list(FsMountList *mounts)
}
}
+static int dev_major_minor(const char *devpath,
+ unsigned int *devmajor, unsigned int *devminor)
+{
+struct stat st;
+
+*devmajor = 0;
+*devminor = 0;
+
+if (stat(devpath, &st) < 0) {
+slog("failed to stat device file '%s': %s", devpath, strerror(errno));
+return -1;
+}
+if (S_ISDIR(st.st_mode)) {
+/* It is bind mount */
+return -2;
+}
+if (S_ISBLK(st.st_mode)) {
+*devmajor = major(st.st_rdev);
+*devminor = minor(st.st_rdev);
+return 0;
+}
+return -1;
+}
+
/*
* Walk the mount table and build a list of local file systems
*/
-static void build_fs_mount_list(FsMountList *mounts, Error **errp)
+static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
{
struct mntent *ment;
FsMount *mount;
char const *mtab = "/proc/self/mounts";
FILE *fp;
+unsigned int devmajor, devminor;
fp = setmntent(mtab, "r");
if (!fp) {
@@ -624,20 +651,368 @@ static void build_fs_mount_list(FsMountList *mounts,
Error **errp)
(strcmp(ment->mnt_type, "cifs") == 0)) {
continue;
}
+if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) == -2) {
+/* Skip bind mounts */
+continue;
+}
mount = g_malloc0(sizeof(FsMount));
mount->dirname = g_strdup(ment->mnt_dir);
mount->devtype = g_strdup(ment->mnt_type);
+mount->devmajor = devmajor;
+mount->devminor = devminor;
QTAILQ_INSERT_TAIL(mounts, mount, next
Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
> On 04/28/2014 09:25 AM, Tomoki Sekiyama wrote:
>> When an array of mount point paths is specified as 'mountpoints' argument
>> of guest-fsfreeze-freeze, qemu-ga with this patch will only freeze the file
>> systems mounted on specified paths in Linux.
>
> How does the management application learn the set of valid mountpoints
> arguments it can pass to this command? Shouldn't there be a query
> counterpart that asks the guest to return the full list of mountpoints
> that it can support freezing? In the case of guests that are
> all-or-none, like Windows, the query command would return an empty list
> to make it obvious there is no ability to freeze just a subset.
>
> In returning a list of mountpoint names, it might also be nice to
> correlate which names map to which devices (it can be a many:many
> mapping, thanks to things like RAID setups in the guest).
I agree.
The patch below is for the command to get filesystems list.
===
From: Tomoki Sekiyama
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.
For example, when `lsblk' result is:
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sdb 8:16 01G 0 disk
`-sdb1 8:17 0 1024M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
sdc 8:32 01G 0 disk
`-sdc1 8:33 0 512M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
vda252:00 25G 0 disk
`-vda1 252:10 25G 0 part /
where sdb is a SCSI disk with PCI controller :00:0a.0 and ID=1,
sdc is an IDE disk with PCI controller :00:01.1, and
vda is a virtio disk with PCI device :00:06.0,
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"},
{"name":"vda1", "mountpoint":"/",
"disk":[
{"bus-type":"virtio","bus":0,"unit":0,"target":0,
"pci-controller":{"bus":0,"slot":6,"domain":0,"function":0}}],
"type":"ext4"}]}
Signed-off-by: Tomoki Sekiyama
---
qga/commands-posix.c | 420 ++-
qga/qapi-schema.json | 77 ++
2 files changed, 496 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 771f00c..212913a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -575,6 +576,7 @@ static void guest_file_init(void)
typedef struct FsMount {
char *dirname;
char *devtype;
+unsigned int devmajor, devminor;
QTAILQ_ENTRY(FsMount) next;
} FsMount;
@@ -596,15 +598,40 @@ static void free_fs_mount_list(FsMountList *mounts)
}
}
+static int dev_major_minor(const char *devpath,
+ unsigned int *devmajor, unsigned int *devminor)
+{
+struct stat st;
+
+*devmajor = 0;
+*devminor = 0;
+
+if (stat(devpath, &st) < 0) {
+slog("failed to stat device file '%s': %s", devpath, strerror(errno));
+return -1;
+}
+if (S_ISDIR(st.st_mode)) {
+/* It is bind mount */
+return -2;
+}
+if (S_ISBLK(st.st_mode)) {
+*devmajor = major(st.st_rdev);
+*devminor = minor(st.st_rdev);
+return 0;
+}
+return -1;
+}
+
/*
* Walk the mount table and build a list of local file systems
*/
-static void build_fs_mount_list(FsMountList *mounts, Error **errp)
+static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
{
struct mntent *ment;
FsMount *mount;
char const *mtab = "/proc/self/mounts";
FILE *fp;
+unsigned int devmajor, devminor;
fp = setmntent(mtab, "r");
if (!fp) {
@@ -624,20 +651,368 @@ static void build_fs_mount_list(FsMountList *mounts,
Error **errp)
(strcmp(ment->mnt_type, "cifs") == 0)) {
continue;
}
+if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) == -2) {
+/* Skip bind mounts */
+continue;
+}
mount = g_malloc0(sizeof(FsMount));
mount->dirname = g_strdup(ment->mnt_dir);
mount->devtype = g_strdup(ment->mnt_type);
+mount->devmajor = devmajor;
+mount->devminor = devminor;
QTAILQ_INSERT_TAIL(mounts, mount, next
Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
On 04/28/2014 09:25 AM, Tomoki Sekiyama wrote: > When an array of mount point paths is specified as 'mountpoints' argument > of guest-fsfreeze-freeze, qemu-ga with this patch will only freeze the file > systems mounted on specified paths in Linux. How does the management application learn the set of valid mountpoints arguments it can pass to this command? Shouldn't there be a query counterpart that asks the guest to return the full list of mountpoints that it can support freezing? In the case of guests that are all-or-none, like Windows, the query command would return an empty list to make it obvious there is no ability to freeze just a subset. In returning a list of mountpoint names, it might also be nice to correlate which names map to which devices (it can be a many:many mapping, thanks to things like RAID setups in the guest). > This would be useful when the host wants to create partial disk snapshots. > > Signed-off-by: Tomoki Sekiyama > --- > qga/commands-posix.c | 17 - > qga/commands-win32.c |3 ++- > qga/qapi-schema.json |4 > 3 files changed, 22 insertions(+), 2 deletions(-) > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
Any comments for this patch? Thanks, Tomoki Sekiyama On 4/28/14 11:25 , "Tomoki Sekiyama" wrote: >Hi, > >This is v2 patch for qemu-ga to add argument to specify which filesystems >to be frozen by guest-fsfreeze-freeze command. > >Changes to v1: > added documentation of the new field > (v1: http://lists.gnu.org/archive/html/qemu-devel/2014-04/msg04085.html) > >--- >Tomoki Sekiyama (1): > qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command > > > qga/commands-posix.c | 17 - > qga/commands-win32.c |3 ++- > qga/qapi-schema.json |4 > 3 files changed, 22 insertions(+), 2 deletions(-) > >-- >Regards, >Tomoki Sekiyama
[Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
Hi, This is v2 patch for qemu-ga to add argument to specify which filesystems to be frozen by guest-fsfreeze-freeze command. Changes to v1: added documentation of the new field (v1: http://lists.gnu.org/archive/html/qemu-devel/2014-04/msg04085.html) --- Tomoki Sekiyama (1): qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command qga/commands-posix.c | 17 - qga/commands-win32.c |3 ++- qga/qapi-schema.json |4 3 files changed, 22 insertions(+), 2 deletions(-) -- Regards, Tomoki Sekiyama
[Qemu-devel] [PATCH v2] qga: Add 'mountpoints' argument to guest-fsfreeze-freeze command
When an array of mount point paths is specified as 'mountpoints' argument
of guest-fsfreeze-freeze, qemu-ga with this patch will only freeze the file
systems mounted on specified paths in Linux.
This would be useful when the host wants to create partial disk snapshots.
Signed-off-by: Tomoki Sekiyama
---
qga/commands-posix.c | 17 -
qga/commands-win32.c |3 ++-
qga/qapi-schema.json |4
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 935a4ec..13efe96 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -711,9 +711,11 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
* Walk list of mounted file systems in the guest, and freeze the ones which
* are real local file systems.
*/
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
+int64_t qmp_guest_fsfreeze_freeze(bool has_mountpoints, strList *mountpoints,
+ Error **err)
{
int ret = 0, i = 0;
+strList *list;
FsMountList mounts;
struct FsMount *mount;
Error *local_err = NULL;
@@ -738,6 +740,19 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
ga_set_frozen(ga_state);
QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
+/* To issue fsfreeze in the reverse order of mounts, check if the
+ * mount is listed in the list here */
+if (has_mountpoints) {
+for (list = mountpoints; list; list = list->next) {
+if (strcmp(list->value, mount->dirname) == 0) {
+break;
+}
+}
+if (!list) {
+continue;
+}
+}
+
fd = qemu_open(mount->dirname, O_RDONLY);
if (fd == -1) {
error_setg_errno(err, errno, "failed to open %s", mount->dirname);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0ee07b6..98dd996 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -173,7 +173,8 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
* Freeze local file systems using Volume Shadow-copy Service.
* The frozen state is limited for up to 10 seconds by VSS.
*/
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
+int64_t qmp_guest_fsfreeze_freeze(bool has_mountpoints, strList *mountpoints,
+ Error **err)
{
int i;
Error *local_err = NULL;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a8cdcb3..31c0dc8 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -378,12 +378,16 @@
#
# Sync and freeze all freezable, local guest filesystems
#
+# @mountpoints: #optional an array of mountpoints of filesystems to be frozen.
+# If omitted, every mounted filesystem is frozen. (Since: 2.1)
+#
# Returns: Number of file systems currently frozen. On error, all filesystems
# will be thawed.
#
# Since: 0.15.0
##
{ 'command': 'guest-fsfreeze-freeze',
+ 'data':{ '*mountpoints': ['str'] },
'returns': 'int' }
##
