* [Qemu-devel] [PATCH V12 01/18] block: move bdrv_snapshot_find() to block/snapshot.c
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot() Wenchao Xia
` (17 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch adds block/snapshot.c and then moves the function
there. It also fixes small code style errors reported by check script.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/Makefile.objs | 1 +
block/snapshot.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
include/block/snapshot.h | 37 +++++++++++++++++++++++++++++++++++
savevm.c | 23 +---------------------
4 files changed, 87 insertions(+), 22 deletions(-)
create mode 100644 block/snapshot.c
create mode 100644 include/block/snapshot.h
diff --git a/block/Makefile.objs b/block/Makefile.objs
index c067f38..60a4cd2 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,6 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
block-obj-y += parallels.o blkdebug.o blkverify.o
+block-obj-y += snapshot.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/snapshot.c b/block/snapshot.c
new file mode 100644
index 0000000..c47a899
--- /dev/null
+++ b/block/snapshot.c
@@ -0,0 +1,48 @@
+/*
+ * Block layer snapshot related functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "block/snapshot.h"
+
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+ const char *name)
+{
+ QEMUSnapshotInfo *sn_tab, *sn;
+ int nb_sns, i, ret;
+
+ ret = -ENOENT;
+ nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+ if (nb_sns < 0) {
+ return ret;
+ }
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
+ *sn_info = *sn;
+ ret = 0;
+ break;
+ }
+ }
+ g_free(sn_tab);
+ return ret;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
new file mode 100644
index 0000000..4ad070c
--- /dev/null
+++ b/include/block/snapshot.h
@@ -0,0 +1,37 @@
+/*
+ * Block layer snapshot related functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef SNAPSHOT_H
+#define SNAPSHOT_H
+
+#include "qemu-common.h"
+/*
+ * block.h is needed for QEMUSnapshotInfo, it can be removed when define is
+ * moved here.
+ */
+#include "block.h"
+
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+ const char *name);
+#endif
diff --git a/savevm.c b/savevm.c
index b1d8988..528ba0d 100644
--- a/savevm.c
+++ b/savevm.c
@@ -40,6 +40,7 @@
#include "trace.h"
#include "qemu/bitops.h"
#include "qemu/iov.h"
+#include "block/snapshot.h"
#define SELF_ANNOUNCE_ROUNDS 5
@@ -2199,28 +2200,6 @@ out:
return ret;
}
-static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
- const char *name)
-{
- QEMUSnapshotInfo *sn_tab, *sn;
- int nb_sns, i, ret;
-
- ret = -ENOENT;
- nb_sns = bdrv_snapshot_list(bs, &sn_tab);
- if (nb_sns < 0)
- return ret;
- for(i = 0; i < nb_sns; i++) {
- sn = &sn_tab[i];
- if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
- *sn_info = *sn;
- ret = 0;
- break;
- }
- }
- g_free(sn_tab);
- return ret;
-}
-
/*
* Deletes snapshots of a given name in all opened images.
*/
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 01/18] block: move bdrv_snapshot_find() to block/snapshot.c Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-17 1:09 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 03/18] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
` (16 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
To make it clear about id and name in searching, the API is changed
a bit to distinguish them, and caller can choose to search by id or name.
Searching will be done with higher priority of id. This function also
returns negative value from bdrv_snapshot_list() instead of -ENOENT on
error now.
Note that the logic is changed a bit: now it traverse twice, first search
for id, second for name, but original code traverse only once to search
them at the same time, so matching sequence may be different. As a result,
do_savevm(), del_existing_snapshots(), load_vmsate() may behaviors differently
if there are unwisely chosen name mixed with id. In do_info_snapshots(),
the caller is changed to search id only, which should be the correct behavior.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/snapshot.c | 46 ++++++++++++++++++++++++++++++++++++++--------
include/block/snapshot.h | 2 +-
savevm.c | 10 +++++-----
3 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index c47a899..d57d04a 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -24,8 +24,20 @@
#include "block/snapshot.h"
+/**
+ * Look up an internal snapshot by @id, or else by @name.
+ * @bs: block device to search
+ * @sn_info: location to store information on the snapshot found
+ * @id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ *
+ * If the snapshot with unique ID @id exists, find it.
+ * Else, if snapshots with name @name exists, find one of them.
+ *
+ * Returns: 0 when a snapshot is found, else -errno.
+ */
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
- const char *name)
+ const char *id, const char *name)
{
QEMUSnapshotInfo *sn_tab, *sn;
int nb_sns, i, ret;
@@ -33,16 +45,34 @@ int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
ret = -ENOENT;
nb_sns = bdrv_snapshot_list(bs, &sn_tab);
if (nb_sns < 0) {
- return ret;
+ return nb_sns;
+ }
+
+ /* search by id */
+ if (id) {
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->id_str, id)) {
+ *sn_info = *sn;
+ ret = 0;
+ goto out;
+ }
+ }
}
- for (i = 0; i < nb_sns; i++) {
- sn = &sn_tab[i];
- if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
- *sn_info = *sn;
- ret = 0;
- break;
+
+ /* search by name */
+ if (name) {
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->name, name)) {
+ *sn_info = *sn;
+ ret = 0;
+ goto out;
+ }
}
}
+
+ out:
g_free(sn_tab);
return ret;
}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 4ad070c..a047a8e 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -33,5 +33,5 @@
#include "block.h"
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
- const char *name);
+ const char *id, const char *name);
#endif
diff --git a/savevm.c b/savevm.c
index 528ba0d..ed6d74c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2212,7 +2212,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
bs = NULL;
while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs) &&
- bdrv_snapshot_find(bs, snapshot, name) >= 0)
+ bdrv_snapshot_find(bs, snapshot, name, name) >= 0)
{
ret = bdrv_snapshot_delete(bs, name);
if (ret < 0) {
@@ -2272,7 +2272,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
if (name) {
- ret = bdrv_snapshot_find(bs, old_sn, name);
+ ret = bdrv_snapshot_find(bs, old_sn, name, name);
if (ret >= 0) {
pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
@@ -2363,7 +2363,7 @@ int load_vmstate(const char *name)
}
/* Don't even try to load empty VM states */
- ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
+ ret = bdrv_snapshot_find(bs_vm_state, &sn, name, name);
if (ret < 0) {
return ret;
} else if (sn.vm_state_size == 0) {
@@ -2387,7 +2387,7 @@ int load_vmstate(const char *name)
return -ENOTSUP;
}
- ret = bdrv_snapshot_find(bs, &sn, name);
+ ret = bdrv_snapshot_find(bs, &sn, name, name);
if (ret < 0) {
error_report("Device '%s' does not have the requested snapshot '%s'",
bdrv_get_device_name(bs), name);
@@ -2493,7 +2493,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
while ((bs1 = bdrv_next(bs1))) {
if (bdrv_can_snapshot(bs1) && bs1 != bs) {
- ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
+ ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
if (ret < 0) {
available = 0;
break;
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot()
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot() Wenchao Xia
@ 2013-04-17 1:09 ` Eric Blake
2013-04-17 18:50 ` Eric Blake
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2013-04-17 1:09 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> To make it clear about id and name in searching, the API is changed
> a bit to distinguish them, and caller can choose to search by id or name.
> Searching will be done with higher priority of id. This function also
> returns negative value from bdrv_snapshot_list() instead of -ENOENT on
> error now.
> Note that the logic is changed a bit: now it traverse twice, first search
> for id, second for name, but original code traverse only once to search
> them at the same time, so matching sequence may be different. As a result,
> do_savevm(), del_existing_snapshots(), load_vmsate() may behaviors differently
> if there are unwisely chosen name mixed with id. In do_info_snapshots(),
> the caller is changed to search id only, which should be the correct behavior.
I just realized that you are trying to do the same thing as Pavel, but
that your two implementations differ.
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03289.html
Rather than spending my time reviewing two competing versions, it would
be really nice if one of you could rebase patches on top of the other,
and present a unified series containing both of your improvements as a
single series, so that we are only changing bdrv_snapshot_list semantics
once.
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot()
2013-04-17 1:09 ` Eric Blake
@ 2013-04-17 18:50 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2013-04-17 18:50 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]
On 04/16/2013 07:09 PM, Eric Blake wrote:
> On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>> To make it clear about id and name in searching, the API is changed
>> a bit to distinguish them, and caller can choose to search by id or name.
>> Searching will be done with higher priority of id. This function also
>> returns negative value from bdrv_snapshot_list() instead of -ENOENT on
>> error now.
>> Note that the logic is changed a bit: now it traverse twice, first search
>> for id, second for name, but original code traverse only once to search
>> them at the same time, so matching sequence may be different. As a result,
>> do_savevm(), del_existing_snapshots(), load_vmsate() may behaviors differently
>> if there are unwisely chosen name mixed with id. In do_info_snapshots(),
>> the caller is changed to search id only, which should be the correct behavior.
>
> I just realized that you are trying to do the same thing as Pavel, but
> that your two implementations differ.
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03289.html
>
> Rather than spending my time reviewing two competing versions, it would
> be really nice if one of you could rebase patches on top of the other,
> and present a unified series containing both of your improvements as a
> single series, so that we are only changing bdrv_snapshot_list semantics
> once.
I ended up reviewing both implementations, and not fully liking either
of them, to the point that I proposed yet a third version. I guess we
need Kevin or Markus to speak up now...
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03501.html
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 03/18] qemu-img: remove unused parameter in collect_image_info()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 01/18] block: move bdrv_snapshot_find() to block/snapshot.c Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 02/18] block: distinguish id and name in bdrv_find_snapshot() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 04/18] block: move collect_snapshots() and collect_image_info() to block/qapi.c Wenchao Xia
` (15 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
Parameter *fmt was not used, so remove it.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 31627b0..937ec01 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1640,8 +1640,7 @@ static void dump_json_image_info(ImageInfo *info)
static void collect_image_info(BlockDriverState *bs,
ImageInfo *info,
- const char *filename,
- const char *fmt)
+ const char *filename)
{
uint64_t total_sectors;
char backing_filename[1024];
@@ -1815,7 +1814,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
}
info = g_new0(ImageInfo, 1);
- collect_image_info(bs, info, filename, fmt);
+ collect_image_info(bs, info, filename);
collect_snapshots(bs, info);
elem = g_new0(ImageInfoList, 1);
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 04/18] block: move collect_snapshots() and collect_image_info() to block/qapi.c
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (2 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 03/18] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 05/18] block: add snapshot info query function bdrv_query_snapshot_info_list() Wenchao Xia
` (14 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch adds block/qapi.c and moves the functions there. To avoid
conflict and tip better, macro in header file is BLOCK_QAPI_H instead
of QAPI_H. The moving is for making review easier, those functions
will be modified and renamed later.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/Makefile.objs | 2 +-
block/qapi.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/qapi.h | 35 ++++++++++++++++
qemu-img.c | 86 +--------------------------------------
4 files changed, 146 insertions(+), 84 deletions(-)
create mode 100644 block/qapi.c
create mode 100644 include/block/qapi.h
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 60a4cd2..fc27bed 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
block-obj-y += parallels.o blkdebug.o blkverify.o
-block-obj-y += snapshot.o
+block-obj-y += snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/qapi.c b/block/qapi.c
new file mode 100644
index 0000000..e2b1b6b
--- /dev/null
+++ b/block/qapi.c
@@ -0,0 +1,107 @@
+/*
+ * Block layer qmp related functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "block/qapi.h"
+#include "block/block_int.h"
+
+void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+{
+ int i, sn_count;
+ QEMUSnapshotInfo *sn_tab = NULL;
+ SnapshotInfoList *info_list, *cur_item = NULL;
+ sn_count = bdrv_snapshot_list(bs, &sn_tab);
+
+ for (i = 0; i < sn_count; i++) {
+ info->has_snapshots = true;
+ info_list = g_new0(SnapshotInfoList, 1);
+
+ info_list->value = g_new0(SnapshotInfo, 1);
+ info_list->value->id = g_strdup(sn_tab[i].id_str);
+ info_list->value->name = g_strdup(sn_tab[i].name);
+ info_list->value->vm_state_size = sn_tab[i].vm_state_size;
+ info_list->value->date_sec = sn_tab[i].date_sec;
+ info_list->value->date_nsec = sn_tab[i].date_nsec;
+ info_list->value->vm_clock_sec = sn_tab[i].vm_clock_nsec / 1000000000;
+ info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+
+ /* XXX: waiting for the qapi to support qemu-queue.h types */
+ if (!cur_item) {
+ info->snapshots = cur_item = info_list;
+ } else {
+ cur_item->next = info_list;
+ cur_item = info_list;
+ }
+
+ }
+
+ g_free(sn_tab);
+}
+
+void bdrv_collect_image_info(BlockDriverState *bs,
+ ImageInfo *info,
+ const char *filename)
+{
+ uint64_t total_sectors;
+ char backing_filename[1024];
+ char backing_filename2[1024];
+ BlockDriverInfo bdi;
+
+ bdrv_get_geometry(bs, &total_sectors);
+
+ info->filename = g_strdup(filename);
+ info->format = g_strdup(bdrv_get_format_name(bs));
+ info->virtual_size = total_sectors * 512;
+ info->actual_size = bdrv_get_allocated_file_size(bs);
+ info->has_actual_size = info->actual_size >= 0;
+ if (bdrv_is_encrypted(bs)) {
+ info->encrypted = true;
+ info->has_encrypted = true;
+ }
+ if (bdrv_get_info(bs, &bdi) >= 0) {
+ if (bdi.cluster_size != 0) {
+ info->cluster_size = bdi.cluster_size;
+ info->has_cluster_size = true;
+ }
+ info->dirty_flag = bdi.is_dirty;
+ info->has_dirty_flag = true;
+ }
+ bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+ if (backing_filename[0] != '\0') {
+ info->backing_filename = g_strdup(backing_filename);
+ info->has_backing_filename = true;
+ bdrv_get_full_backing_filename(bs, backing_filename2,
+ sizeof(backing_filename2));
+
+ if (strcmp(backing_filename, backing_filename2) != 0) {
+ info->full_backing_filename =
+ g_strdup(backing_filename2);
+ info->has_full_backing_filename = true;
+ }
+
+ if (bs->backing_format[0]) {
+ info->backing_filename_format = g_strdup(bs->backing_format);
+ info->has_backing_filename_format = true;
+ }
+ }
+}
diff --git a/include/block/qapi.h b/include/block/qapi.h
new file mode 100644
index 0000000..4586578
--- /dev/null
+++ b/include/block/qapi.h
@@ -0,0 +1,35 @@
+/*
+ * Block layer qmp related functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef BLOCK_QAPI_H
+#define BLOCK_QAPI_H
+
+#include "qapi-types.h"
+#include "block/block.h"
+
+void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info);
+void bdrv_collect_image_info(BlockDriverState *bs,
+ ImageInfo *info,
+ const char *filename);
+#endif
diff --git a/qemu-img.c b/qemu-img.c
index 937ec01..a020ccc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -30,6 +30,7 @@
#include "qemu/osdep.h"
#include "sysemu/sysemu.h"
#include "block/block_int.h"
+#include "block/qapi.h"
#include <getopt.h>
#include <stdio.h>
#include <stdarg.h>
@@ -1588,39 +1589,6 @@ static void dump_json_image_info_list(ImageInfoList *list)
QDECREF(str);
}
-static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
-{
- int i, sn_count;
- QEMUSnapshotInfo *sn_tab = NULL;
- SnapshotInfoList *info_list, *cur_item = NULL;
- sn_count = bdrv_snapshot_list(bs, &sn_tab);
-
- for (i = 0; i < sn_count; i++) {
- info->has_snapshots = true;
- info_list = g_new0(SnapshotInfoList, 1);
-
- info_list->value = g_new0(SnapshotInfo, 1);
- info_list->value->id = g_strdup(sn_tab[i].id_str);
- info_list->value->name = g_strdup(sn_tab[i].name);
- info_list->value->vm_state_size = sn_tab[i].vm_state_size;
- info_list->value->date_sec = sn_tab[i].date_sec;
- info_list->value->date_nsec = sn_tab[i].date_nsec;
- info_list->value->vm_clock_sec = sn_tab[i].vm_clock_nsec / 1000000000;
- info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
-
- /* XXX: waiting for the qapi to support qemu-queue.h types */
- if (!cur_item) {
- info->snapshots = cur_item = info_list;
- } else {
- cur_item->next = info_list;
- cur_item = info_list;
- }
-
- }
-
- g_free(sn_tab);
-}
-
static void dump_json_image_info(ImageInfo *info)
{
Error *errp = NULL;
@@ -1638,54 +1606,6 @@ static void dump_json_image_info(ImageInfo *info)
QDECREF(str);
}
-static void collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename)
-{
- uint64_t total_sectors;
- char backing_filename[1024];
- char backing_filename2[1024];
- BlockDriverInfo bdi;
-
- bdrv_get_geometry(bs, &total_sectors);
-
- info->filename = g_strdup(filename);
- info->format = g_strdup(bdrv_get_format_name(bs));
- info->virtual_size = total_sectors * 512;
- info->actual_size = bdrv_get_allocated_file_size(bs);
- info->has_actual_size = info->actual_size >= 0;
- if (bdrv_is_encrypted(bs)) {
- info->encrypted = true;
- info->has_encrypted = true;
- }
- if (bdrv_get_info(bs, &bdi) >= 0) {
- if (bdi.cluster_size != 0) {
- info->cluster_size = bdi.cluster_size;
- info->has_cluster_size = true;
- }
- info->dirty_flag = bdi.is_dirty;
- info->has_dirty_flag = true;
- }
- bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
- if (backing_filename[0] != '\0') {
- info->backing_filename = g_strdup(backing_filename);
- info->has_backing_filename = true;
- bdrv_get_full_backing_filename(bs, backing_filename2,
- sizeof(backing_filename2));
-
- if (strcmp(backing_filename, backing_filename2) != 0) {
- info->full_backing_filename =
- g_strdup(backing_filename2);
- info->has_full_backing_filename = true;
- }
-
- if (bs->backing_format[0]) {
- info->backing_filename_format = g_strdup(bs->backing_format);
- info->has_backing_filename_format = true;
- }
- }
-}
-
static void dump_human_image_info(ImageInfo *info)
{
char size_buf[128], dsize_buf[128];
@@ -1814,8 +1734,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
}
info = g_new0(ImageInfo, 1);
- collect_image_info(bs, info, filename);
- collect_snapshots(bs, info);
+ bdrv_collect_image_info(bs, info, filename);
+ bdrv_collect_snapshots(bs, info);
elem = g_new0(ImageInfoList, 1);
elem->value = info;
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 05/18] block: add snapshot info query function bdrv_query_snapshot_info_list()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (3 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 04/18] block: move collect_snapshots() and collect_image_info() to block/qapi.c Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list() Wenchao Xia
` (13 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch adds function bdrv_query_snapshot_info_list(), which will
retrieve snapshot info of an image in qmp object format. The implementation
is based on the code moved from qemu-img.c with modification to fit more
for qmp based block layer API.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qapi.c | 56 ++++++++++++++++++++++++++++++++++++++-----------
include/block/qapi.h | 4 ++-
qemu-img.c | 5 +++-
3 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index e2b1b6b..9bfa547 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,29 +25,57 @@
#include "block/qapi.h"
#include "block/block_int.h"
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+/*
+ * Returns 0 on success, with *p_list either set to describe snapshot
+ * information, or NULL because there are no snapshots. Returns -errno on
+ * error, with *p_list untouched.
+ */
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+ SnapshotInfoList **p_list,
+ Error **errp)
{
int i, sn_count;
QEMUSnapshotInfo *sn_tab = NULL;
- SnapshotInfoList *info_list, *cur_item = NULL;
+ SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
+ SnapshotInfo *info;
+
sn_count = bdrv_snapshot_list(bs, &sn_tab);
+ if (sn_count < 0) {
+ const char *dev = bdrv_get_device_name(bs);
+ switch (sn_count) {
+ case -ENOMEDIUM:
+ error_setg(errp, "Device '%s' is not inserted", dev);
+ break;
+ case -ENOTSUP:
+ error_setg(errp,
+ "Device '%s' does not support internal snapshots",
+ dev);
+ break;
+ default:
+ error_setg_errno(errp, -sn_count,
+ "Can't list snapshots of device '%s'", dev);
+ break;
+ }
+ return sn_count;
+ }
for (i = 0; i < sn_count; i++) {
- info->has_snapshots = true;
- info_list = g_new0(SnapshotInfoList, 1);
- info_list->value = g_new0(SnapshotInfo, 1);
- info_list->value->id = g_strdup(sn_tab[i].id_str);
- info_list->value->name = g_strdup(sn_tab[i].name);
- info_list->value->vm_state_size = sn_tab[i].vm_state_size;
- info_list->value->date_sec = sn_tab[i].date_sec;
- info_list->value->date_nsec = sn_tab[i].date_nsec;
- info_list->value->vm_clock_sec = sn_tab[i].vm_clock_nsec / 1000000000;
- info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+ info = g_new0(SnapshotInfo, 1);
+ info->id = g_strdup(sn_tab[i].id_str);
+ info->name = g_strdup(sn_tab[i].name);
+ info->vm_state_size = sn_tab[i].vm_state_size;
+ info->date_sec = sn_tab[i].date_sec;
+ info->date_nsec = sn_tab[i].date_nsec;
+ info->vm_clock_sec = sn_tab[i].vm_clock_nsec / 1000000000;
+ info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+
+ info_list = g_new0(SnapshotInfoList, 1);
+ info_list->value = info;
/* XXX: waiting for the qapi to support qemu-queue.h types */
if (!cur_item) {
- info->snapshots = cur_item = info_list;
+ head = cur_item = info_list;
} else {
cur_item->next = info_list;
cur_item = info_list;
@@ -56,6 +84,8 @@ void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
}
g_free(sn_tab);
+ *p_list = head;
+ return 0;
}
void bdrv_collect_image_info(BlockDriverState *bs,
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 4586578..91dc41b 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -28,7 +28,9 @@
#include "qapi-types.h"
#include "block/block.h"
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info);
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+ SnapshotInfoList **p_list,
+ Error **errp);
void bdrv_collect_image_info(BlockDriverState *bs,
ImageInfo *info,
const char *filename);
diff --git a/qemu-img.c b/qemu-img.c
index a020ccc..472b264 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1735,7 +1735,10 @@ static ImageInfoList *collect_image_info_list(const char *filename,
info = g_new0(ImageInfo, 1);
bdrv_collect_image_info(bs, info, filename);
- bdrv_collect_snapshots(bs, info);
+ bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL);
+ if (info->snapshots) {
+ info->has_snapshots = true;
+ }
elem = g_new0(ImageInfoList, 1);
elem->value = info;
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (4 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 05/18] block: add snapshot info query function bdrv_query_snapshot_info_list() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-17 20:52 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic Wenchao Xia
` (12 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch adds a parameter to tell whether return valid snapshots
for whole VM only.
Note that the snapshot check logic is copied from do_info_snapshots(),
which is different with load_vmstate() and will be changed in next patch.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/qapi.c | 39 +++++++++++++++++++++++++++++++++++++--
include/block/qapi.h | 1 +
qemu-img.c | 2 +-
3 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 9bfa547..97c5cd4 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -23,15 +23,48 @@
*/
#include "block/qapi.h"
+#include "block/snapshot.h"
#include "block/block_int.h"
/*
+ * check whether the snapshot is valid for whole vm.
+ *
+ * @sn: snapshot info to be checked.
+ * @bs: where @sn was found.
+ *
+ * return true if the snapshot is consistent for the VM.
+ */
+static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
+ BlockDriverState *bs)
+{
+ BlockDriverState *bs1 = NULL;
+ QEMUSnapshotInfo s, *sn_info = &s;
+ int ret;
+
+ /* Check logic is connected with load_vmstate():
+ Only check the devices that can snapshot, other devices that can't
+ take snapshot, for example, readonly ones, will be ignored in
+ load_vmstate(). */
+ while ((bs1 = bdrv_next(bs1))) {
+ if (bs1 != bs && bdrv_can_snapshot(bs1)) {
+ ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
+ if (ret < 0) {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
+/*
* Returns 0 on success, with *p_list either set to describe snapshot
* information, or NULL because there are no snapshots. Returns -errno on
- * error, with *p_list untouched.
+ * error, with *p_list untouched. If @vm_snapshot is true, limit the results
+ * to snapshots valid for the whole VM.
*/
int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
+ bool vm_snapshot,
Error **errp)
{
int i, sn_count;
@@ -60,7 +93,9 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
}
for (i = 0; i < sn_count; i++) {
-
+ if (vm_snapshot && !snapshot_valid_for_vm(&sn_tab[i], bs)) {
+ continue;
+ }
info = g_new0(SnapshotInfo, 1);
info->id = g_strdup(sn_tab[i].id_str);
info->name = g_strdup(sn_tab[i].name);
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 91dc41b..fe66053 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -30,6 +30,7 @@
int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
+ bool vm_snapshot,
Error **errp);
void bdrv_collect_image_info(BlockDriverState *bs,
ImageInfo *info,
diff --git a/qemu-img.c b/qemu-img.c
index 472b264..f537014 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1735,7 +1735,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
info = g_new0(ImageInfo, 1);
bdrv_collect_image_info(bs, info, filename);
- bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL);
+ bdrv_query_snapshot_info_list(bs, &info->snapshots, false, NULL);
if (info->snapshots) {
info->has_snapshots = true;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list()
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list() Wenchao Xia
@ 2013-04-17 20:52 ` Eric Blake
2013-04-18 4:07 ` Wenchao Xia
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2013-04-17 20:52 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> This patch adds a parameter to tell whether return valid snapshots
> for whole VM only.
> Note that the snapshot check logic is copied from do_info_snapshots(),
> which is different with load_vmstate() and will be changed in next patch.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
> +
> + /* Check logic is connected with load_vmstate():
> + Only check the devices that can snapshot, other devices that can't
> + take snapshot, for example, readonly ones, will be ignored in
> + load_vmstate(). */
> + while ((bs1 = bdrv_next(bs1))) {
> + if (bs1 != bs && bdrv_can_snapshot(bs1)) {
> + ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
This says for a snapshot to be consistent, all block devices must share
the same id but can have different names. Is that really true? Or is
it backwards from reality? If snapshot ids allocated incrementally per
block device, can I use hotplug to create a situation where I have a VM
with two disks
disk a has snapshot id 1 named 'A', id 2 named 'B'
disk b has snapshot id 1 named 'B'
where the existing HMP 'loadvm B' should load the snapshot named 'B'
from both disks, regardless of the different number, and where snapshot
'A' is inconsistent unless disk b is hot-unplugged?
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list()
2013-04-17 20:52 ` Eric Blake
@ 2013-04-18 4:07 ` Wenchao Xia
0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-18 4:07 UTC (permalink / raw)
To: Eric Blake
Cc: kwolf, phrdina, qemu-devel, stefanha, armbru, lcapitulino,
pbonzini
于 2013-4-18 4:52, Eric Blake 写道:
> On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>> This patch adds a parameter to tell whether return valid snapshots
>> for whole VM only.
>> Note that the snapshot check logic is copied from do_info_snapshots(),
>> which is different with load_vmstate() and will be changed in next patch.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>
>> +
>> + /* Check logic is connected with load_vmstate():
>> + Only check the devices that can snapshot, other devices that can't
>> + take snapshot, for example, readonly ones, will be ignored in
>> + load_vmstate(). */
>> + while ((bs1 = bdrv_next(bs1))) {
>> + if (bs1 != bs && bdrv_can_snapshot(bs1)) {
>> + ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
>
> This says for a snapshot to be consistent, all block devices must share
> the same id but can have different names. Is that really true? Or is
> it backwards from reality? If snapshot ids allocated incrementally per
> block device, can I use hotplug to create a situation where I have a VM
> with two disks
>
OK, it would check both.
> where the existing HMP 'loadvm B' should load the snapshot named 'B'
> from both disks, regardless of the different number, and where snapshot
> 'A' is inconsistent unless disk b is hot-unplugged?
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (5 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 06/18] block: add check for VM snapshot in bdrv_query_snapshot_info_list() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-17 21:58 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info() Wenchao Xia
` (11 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
Original logic is different with load_vmstate(), this patch change it
to be exactly the same with load_vmstate(), so any VM snapshot shown in
qmp/hmp should succeed in load_vmstate().
Note that, runtime snapshot info maybe different with what is got
in "qemu-img info" as static snapshot info, and this patch clearly
tips it.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 97c5cd4..49c0eb0 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -46,7 +46,18 @@ static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
take snapshot, for example, readonly ones, will be ignored in
load_vmstate(). */
while ((bs1 = bdrv_next(bs1))) {
- if (bs1 != bs && bdrv_can_snapshot(bs1)) {
+
+ if (!bdrv_is_inserted(bs1) || bdrv_is_read_only(bs1)) {
+ continue;
+ }
+
+ if (!bdrv_can_snapshot(bs1)) {
+ /* Device is writable but does not support snapshots, will be
+ rejected by load_vmstate(). */
+ return false;
+ }
+
+ if (bs1 != bs) {
ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
if (ret < 0) {
return false;
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic Wenchao Xia
@ 2013-04-17 21:58 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2013-04-17 21:58 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> Original logic is different with load_vmstate(), this patch change it
s/with/from/
s/change/changes/
> to be exactly the same with load_vmstate(), so any VM snapshot shown in
s/with/as/
> qmp/hmp should succeed in load_vmstate().
Looking through git logs, most people use blank lines and start at
column 1, rather than your idiom of 2-space indent and no blank line,
when starting a new paragraph.
> Note that, runtime snapshot info maybe different with what is got
s/that,/that consistent/
s/maybe different with/may be a subset of/
s/got/listed/
> in "qemu-img info" as static snapshot info, and this patch clearly
> tips it.
s/tips/identifies/
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> block/qapi.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 97c5cd4..49c0eb0 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -46,7 +46,18 @@ static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
> take snapshot, for example, readonly ones, will be ignored in
> load_vmstate(). */
> while ((bs1 = bdrv_next(bs1))) {
> - if (bs1 != bs && bdrv_can_snapshot(bs1)) {
> +
A blank line after a while(){ is not common, but as it is only
whitespace, and checkpatch didn't complain, it doesn't hold up my review.
> + if (!bdrv_is_inserted(bs1) || bdrv_is_read_only(bs1)) {
> + continue;
> + }
> +
> + if (!bdrv_can_snapshot(bs1)) {
> + /* Device is writable but does not support snapshots, will be
> + rejected by load_vmstate(). */
> + return false;
> + }
> +
> + if (bs1 != bs) {
> ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
> if (ret < 0) {
> return false;
>
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (6 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 07/18] block: change VM snapshot checking logic Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-17 22:56 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 09/18] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c Wenchao Xia
` (10 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch adds function bdrv_query_image_info(), which will
retrieve image info in qmp object format. The implementation is
based on the code moved from qemu-img.c, but uses block layer
function to get snapshot info.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 37 +++++++++++++++++++++++++++++++------
include/block/qapi.h | 6 +++---
qemu-img.c | 7 ++-----
3 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 49c0eb0..ac0ab57 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -134,18 +134,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
return 0;
}
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename)
+/* return 0 on success, and @p_info will be set only on success. */
+int bdrv_query_image_info(BlockDriverState *bs,
+ ImageInfo **p_info,
+ Error **errp)
{
uint64_t total_sectors;
- char backing_filename[1024];
+ const char *backing_filename;
char backing_filename2[1024];
BlockDriverInfo bdi;
+ int ret;
+ Error *err = NULL;
+ ImageInfo *info = g_new0(ImageInfo, 1);
bdrv_get_geometry(bs, &total_sectors);
- info->filename = g_strdup(filename);
+ info->filename = g_strdup(bs->filename);
info->format = g_strdup(bdrv_get_format_name(bs));
info->virtual_size = total_sectors * 512;
info->actual_size = bdrv_get_allocated_file_size(bs);
@@ -162,7 +166,7 @@ void bdrv_collect_image_info(BlockDriverState *bs,
info->dirty_flag = bdi.is_dirty;
info->has_dirty_flag = true;
}
- bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+ backing_filename = bs->backing_file;
if (backing_filename[0] != '\0') {
info->backing_filename = g_strdup(backing_filename);
info->has_backing_filename = true;
@@ -180,4 +184,25 @@ void bdrv_collect_image_info(BlockDriverState *bs,
info->has_backing_filename_format = true;
}
}
+
+ ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
+ switch (ret) {
+ case 0:
+ if (info->snapshots) {
+ info->has_snapshots = true;
+ }
+ break;
+ /* recoverable error */
+ case -ENOMEDIUM:
+ case -ENOTSUP:
+ error_free(err);
+ break;
+ default:
+ error_propagate(errp, err);
+ qapi_free_ImageInfo(info);
+ return ret;
+ }
+
+ *p_info = info;
+ return 0;
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index fe66053..2c62fdf 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
bool vm_snapshot,
Error **errp);
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename);
+int bdrv_query_image_info(BlockDriverState *bs,
+ ImageInfo **p_info,
+ Error **errp);
#endif
diff --git a/qemu-img.c b/qemu-img.c
index f537014..1dd0a60 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1733,11 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
goto err;
}
- info = g_new0(ImageInfo, 1);
- bdrv_collect_image_info(bs, info, filename);
- bdrv_query_snapshot_info_list(bs, &info->snapshots, false, NULL);
- if (info->snapshots) {
- info->has_snapshots = true;
+ if (bdrv_query_image_info(bs, &info, NULL)) {
+ goto err;
}
elem = g_new0(ImageInfoList, 1);
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info()
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info() Wenchao Xia
@ 2013-04-17 22:56 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2013-04-17 22:56 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> This patch adds function bdrv_query_image_info(), which will
> retrieve image info in qmp object format. The implementation is
> based on the code moved from qemu-img.c, but uses block layer
> function to get snapshot info.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> block/qapi.c | 37 +++++++++++++++++++++++++++++++------
> include/block/qapi.h | 6 +++---
> qemu-img.c | 7 ++-----
> 3 files changed, 36 insertions(+), 14 deletions(-)
>
> +/* return 0 on success, and @p_info will be set only on success. */
> +int bdrv_query_image_info(BlockDriverState *bs,
> + ImageInfo **p_info,
> + Error **errp)
Is it necessary to return a value? If all callers pass in an errp, then
this function can be void, and you can use the error parameter as the
lone indication of problems.
> +++ b/qemu-img.c
> @@ -1733,11 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
> goto err;
> }
>
> - info = g_new0(ImageInfo, 1);
> - bdrv_collect_image_info(bs, info, filename);
> - bdrv_query_snapshot_info_list(bs, &info->snapshots, false, NULL);
> - if (info->snapshots) {
> - info->has_snapshots = true;
> + if (bdrv_query_image_info(bs, &info, NULL)) {
> + goto err;
But then this caller would need to pass in a local error parameter,
where Pavel's idea of qemu_img_handle_error would make it easier.
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03288.html
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 09/18] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (7 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 10/18] qmp: add interface query-snapshots Wenchao Xia
` (9 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This is a code move patch, except in qmp_query_block bdrv_next(bs)
is used instead of direct traverse of global array 'bdrv_states'.
This patch also fix code style error reported by check script.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 76 ------------------------------------------------
block/qapi.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 -
include/block/qapi.h | 1 +
4 files changed, 78 insertions(+), 77 deletions(-)
diff --git a/block.c b/block.c
index 602d8a4..126cd4b 100644
--- a/block.c
+++ b/block.c
@@ -3016,82 +3016,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
return data.ret;
}
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
-{
- BlockInfo *info = g_malloc0(sizeof(*info));
- info->device = g_strdup(bs->device_name);
- info->type = g_strdup("unknown");
- info->locked = bdrv_dev_is_medium_locked(bs);
- info->removable = bdrv_dev_has_removable_media(bs);
-
- if (bdrv_dev_has_removable_media(bs)) {
- info->has_tray_open = true;
- info->tray_open = bdrv_dev_is_tray_open(bs);
- }
-
- if (bdrv_iostatus_is_enabled(bs)) {
- info->has_io_status = true;
- info->io_status = bs->iostatus;
- }
-
- if (bs->dirty_bitmap) {
- info->has_dirty = true;
- info->dirty = g_malloc0(sizeof(*info->dirty));
- info->dirty->count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE;
- info->dirty->granularity =
- ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bs->dirty_bitmap));
- }
-
- if (bs->drv) {
- info->has_inserted = true;
- info->inserted = g_malloc0(sizeof(*info->inserted));
- info->inserted->file = g_strdup(bs->filename);
- info->inserted->ro = bs->read_only;
- info->inserted->drv = g_strdup(bs->drv->format_name);
- info->inserted->encrypted = bs->encrypted;
- info->inserted->encryption_key_missing = bdrv_key_required(bs);
-
- if (bs->backing_file[0]) {
- info->inserted->has_backing_file = true;
- info->inserted->backing_file = g_strdup(bs->backing_file);
- }
-
- info->inserted->backing_file_depth = bdrv_get_backing_file_depth(bs);
-
- if (bs->io_limits_enabled) {
- info->inserted->bps =
- bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
- info->inserted->bps_rd =
- bs->io_limits.bps[BLOCK_IO_LIMIT_READ];
- info->inserted->bps_wr =
- bs->io_limits.bps[BLOCK_IO_LIMIT_WRITE];
- info->inserted->iops =
- bs->io_limits.iops[BLOCK_IO_LIMIT_TOTAL];
- info->inserted->iops_rd =
- bs->io_limits.iops[BLOCK_IO_LIMIT_READ];
- info->inserted->iops_wr =
- bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE];
- }
- }
- return info;
-}
-
-BlockInfoList *qmp_query_block(Error **errp)
-{
- BlockInfoList *head = NULL, **p_next = &head;
- BlockDriverState *bs;
-
- QTAILQ_FOREACH(bs, &bdrv_states, list) {
- BlockInfoList *info = g_malloc0(sizeof(*info));
- info->value = bdrv_query_info(bs);
-
- *p_next = info;
- p_next = &info->next;
- }
-
- return head;
-}
-
BlockStats *bdrv_query_stats(const BlockDriverState *bs)
{
BlockStats *s;
diff --git a/block/qapi.c b/block/qapi.c
index ac0ab57..73c1961 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,6 +25,7 @@
#include "block/qapi.h"
#include "block/snapshot.h"
#include "block/block_int.h"
+#include "qmp-commands.h"
/*
* check whether the snapshot is valid for whole vm.
@@ -206,3 +207,79 @@ int bdrv_query_image_info(BlockDriverState *bs,
*p_info = info;
return 0;
}
+
+BlockInfo *bdrv_query_info(BlockDriverState *bs)
+{
+ BlockInfo *info = g_malloc0(sizeof(*info));
+ info->device = g_strdup(bs->device_name);
+ info->type = g_strdup("unknown");
+ info->locked = bdrv_dev_is_medium_locked(bs);
+ info->removable = bdrv_dev_has_removable_media(bs);
+
+ if (bdrv_dev_has_removable_media(bs)) {
+ info->has_tray_open = true;
+ info->tray_open = bdrv_dev_is_tray_open(bs);
+ }
+
+ if (bdrv_iostatus_is_enabled(bs)) {
+ info->has_io_status = true;
+ info->io_status = bs->iostatus;
+ }
+
+ if (bs->dirty_bitmap) {
+ info->has_dirty = true;
+ info->dirty = g_malloc0(sizeof(*info->dirty));
+ info->dirty->count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE;
+ info->dirty->granularity =
+ ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bs->dirty_bitmap));
+ }
+
+ if (bs->drv) {
+ info->has_inserted = true;
+ info->inserted = g_malloc0(sizeof(*info->inserted));
+ info->inserted->file = g_strdup(bs->filename);
+ info->inserted->ro = bs->read_only;
+ info->inserted->drv = g_strdup(bs->drv->format_name);
+ info->inserted->encrypted = bs->encrypted;
+ info->inserted->encryption_key_missing = bdrv_key_required(bs);
+
+ if (bs->backing_file[0]) {
+ info->inserted->has_backing_file = true;
+ info->inserted->backing_file = g_strdup(bs->backing_file);
+ }
+
+ info->inserted->backing_file_depth = bdrv_get_backing_file_depth(bs);
+
+ if (bs->io_limits_enabled) {
+ info->inserted->bps =
+ bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
+ info->inserted->bps_rd =
+ bs->io_limits.bps[BLOCK_IO_LIMIT_READ];
+ info->inserted->bps_wr =
+ bs->io_limits.bps[BLOCK_IO_LIMIT_WRITE];
+ info->inserted->iops =
+ bs->io_limits.iops[BLOCK_IO_LIMIT_TOTAL];
+ info->inserted->iops_rd =
+ bs->io_limits.iops[BLOCK_IO_LIMIT_READ];
+ info->inserted->iops_wr =
+ bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE];
+ }
+ }
+ return info;
+}
+
+BlockInfoList *qmp_query_block(Error **errp)
+{
+ BlockInfoList *head = NULL, **p_next = &head;
+ BlockDriverState *bs = NULL;
+
+ while ((bs = bdrv_next(bs))) {
+ BlockInfoList *info = g_malloc0(sizeof(*info));
+ info->value = bdrv_query_info(bs);
+
+ *p_next = info;
+ p_next = &info->next;
+ }
+
+ return head;
+}
diff --git a/include/block/block.h b/include/block/block.h
index 9dc6aad..a319f0a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -326,7 +326,6 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
void bdrv_get_full_backing_filename(BlockDriverState *bs,
char *dest, size_t sz);
-BlockInfo *bdrv_query_info(BlockDriverState *s);
BlockStats *bdrv_query_stats(const BlockDriverState *bs);
int bdrv_can_snapshot(BlockDriverState *bs);
int bdrv_is_snapshot(BlockDriverState *bs);
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 2c62fdf..0039a70 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -35,4 +35,5 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
int bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
Error **errp);
+BlockInfo *bdrv_query_info(BlockDriverState *bs);
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 10/18] qmp: add interface query-snapshots
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (8 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 09/18] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-17 23:03 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 11/18] qmp: add recursive member in ImageInfo Wenchao Xia
` (8 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This interface returns info of valid internal snapshots for whole vm.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 17 +++++++++++++++
qapi-schema.json | 14 +++++++++++++
qmp-commands.hx | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 73c1961..13c7860 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -268,6 +268,23 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
return info;
}
+SnapshotInfoList *qmp_query_snapshots(Error **errp)
+{
+ BlockDriverState *bs;
+ SnapshotInfoList *list = NULL;
+
+ /* internal snapshot for whole vm */
+ bs = bdrv_snapshots();
+ if (!bs) {
+ error_setg(errp, "No available block device supports snapshots\n");
+ return NULL;
+ }
+
+ bdrv_query_snapshot_info_list(bs, &list, true, errp);
+
+ return list;
+}
+
BlockInfoList *qmp_query_block(Error **errp)
{
BlockInfoList *head = NULL, **p_next = &head;
diff --git a/qapi-schema.json b/qapi-schema.json
index db542f6..16d0b1f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -841,6 +841,20 @@
{ 'command': 'query-block', 'returns': ['BlockInfo'] }
##
+# @query-snapshots:
+#
+# Get a list of internal snapshots for the whole virtual machine. Only valid
+# internal snapshots will be returned, inconsistent ones will be ignored
+#
+# Returns: a list of @SnapshotInfo describing all consistent virtual machine
+# snapshots
+#
+# Since: 1.5
+##
+{ 'command': 'query-snapshots',
+ 'returns': ['SnapshotInfo'] }
+
+##
# @BlockDeviceStats:
#
# Statistics of a virtual block device or a block backing device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1e0e11e..e13c029 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1765,6 +1765,64 @@ EQMP
},
SQMP
+query-snapshots
+---------------
+
+Show the internal consistent snapshot information
+
+Consistent snapshots are snapshots that exist in all writeable block devices.
+When 'savevm' takes a snapshot it uses the same ID across all writeable block
+devices. If a snapshot ID only exists in one block device then it is not a
+consistent one. Each snapshot is represented by a json-object and the returned
+value is a json-array of all snapshots
+
+Each json-object contain the following:
+
+- "id": unique snapshot id (json-string)
+- "name": internal snapshot name (json-string)
+- "vm-state-size": size of the VM state in bytes (json-int)
+- "date-sec": UTC date of the snapshot in seconds (json-int)
+- "date-nsec": fractional part in nanoseconds to be used with
+ date-sec(json-int)
+- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
+- "vm-clock-nsec": fractional part in nanoseconds to be used with
+ vm-clock-sec (json-int)
+
+Example:
+
+-> { "execute": "query-snapshots" }
+<- {
+ "return":[
+ {
+ "id": "1",
+ "name": "snapshot1",
+ "vm-state-size": 0,
+ "date-sec": 10000200,
+ "date-nsec": 12,
+ "vm-clock-sec": 206,
+ "vm-clock-nsec": 30
+ },
+ {
+ "id": "2",
+ "name": "snapshot2",
+ "vm-state-size": 34000000,
+ "date-sec": 13000200,
+ "date-nsec": 32,
+ "vm-clock-sec": 406,
+ "vm-clock-nsec": 31
+ }
+ ]
+ }
+
+EQMP
+
+ {
+ .name = "query-snapshots",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
+ },
+
+SQMP
query-blockstats
----------------
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 10/18] qmp: add interface query-snapshots
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 10/18] qmp: add interface query-snapshots Wenchao Xia
@ 2013-04-17 23:03 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2013-04-17 23:03 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> This interface returns info of valid internal snapshots for whole vm.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> block/qapi.c | 17 +++++++++++++++
> qapi-schema.json | 14 +++++++++++++
> qmp-commands.hx | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 89 insertions(+), 0 deletions(-)
> +query-snapshots
> +---------------
> +
> +Show the internal consistent snapshot information
> +
> +Consistent snapshots are snapshots that exist in all writeable block devices.
> +When 'savevm' takes a snapshot it uses the same ID across all writeable block
> +devices. If a snapshot ID only exists in one block device then it is not a
> +consistent one. Each snapshot is represented by a json-object and the returned
> +value is a json-array of all snapshots
Does it really share the same snapshot ID across all block devices, or
is it just the snapshot tag that gets shared? That is, if I have just
disk a, take one snapshot, then hotplug disk b, then take another
snapshot, doesn't that leave me with:
disk a: id 1 tag A, id 2 tag B
disk b: id 1 tag B?
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 11/18] qmp: add recursive member in ImageInfo
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (9 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 10/18] qmp: add interface query-snapshots Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 12/18] qmp: add ImageInfo in BlockDeviceInfo used by query-block Wenchao Xia
` (7 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
New member *backing-image is added to reflect the backing chain
status.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 18 +++++++++++++++++-
qapi-schema.json | 5 ++++-
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 13c7860..2f33a05 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -135,7 +135,23 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
return 0;
}
-/* return 0 on success, and @p_info will be set only on success. */
+/**
+ * bdrv_query_image_info:
+ * @bs: block device to examine
+ * @p_info: location to store image information
+ * @errp: location to store error information
+ *
+ * Store "flat" image inforation in @p_info.
+ *
+ * "Flat" means it does *not* query backing image information,
+ * i.e. (*pinfo)->has_backing_image will be set to false and
+ * (*pinfo)->backing_image to NULL even when the image does in fact have
+ * a backing image.
+ *
+ * On error, store error in @errp.
+ *
+ * Returns: 0 on success, -errno on error.
+ */
int bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 16d0b1f..955ff05 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -233,6 +233,8 @@
#
# @snapshots: #optional list of VM snapshots
#
+# @backing-image: #optional info of the backing image (since 1.5)
+#
# Since: 1.3
#
##
@@ -242,7 +244,8 @@
'*actual-size': 'int', 'virtual-size': 'int',
'*cluster-size': 'int', '*encrypted': 'bool',
'*backing-filename': 'str', '*full-backing-filename': 'str',
- '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
+ '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
+ '*backing-image': 'ImageInfo' } }
##
# @ImageCheck:
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 12/18] qmp: add ImageInfo in BlockDeviceInfo used by query-block
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (10 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 11/18] qmp: add recursive member in ImageInfo Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 13/18] block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c Wenchao Xia
` (6 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
Now image info will be retrieved as an embbed json object inside
BlockDeviceInfo, backing chain info and all related internal snapshot
info can be got in the enhanced recursive structure of ImageInfo.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 40 ++++++++++++++++++++++++++--
include/block/qapi.h | 4 ++-
qapi-schema.json | 5 +++-
qmp-commands.hx | 69 ++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 111 insertions(+), 7 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 2f33a05..24386b4 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -224,9 +224,14 @@ int bdrv_query_image_info(BlockDriverState *bs,
return 0;
}
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
+/* @p_info will be set only on success. */
+void bdrv_query_info(BlockDriverState *bs,
+ BlockInfo **p_info,
+ Error **errp)
{
BlockInfo *info = g_malloc0(sizeof(*info));
+ BlockDriverState *bs0;
+ ImageInfo **p_image_info;
info->device = g_strdup(bs->device_name);
info->type = g_strdup("unknown");
info->locked = bdrv_dev_is_medium_locked(bs);
@@ -280,8 +285,28 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
info->inserted->iops_wr =
bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE];
}
+
+ bs0 = bs;
+ p_image_info = &info->inserted->image;
+ while (1) {
+ if (bdrv_query_image_info(bs0, p_image_info, errp)) {
+ goto err;
+ }
+ if (bs0->drv && bs0->backing_hd) {
+ bs0 = bs0->backing_hd;
+ (*p_image_info)->has_backing_image = true;
+ p_image_info = &((*p_image_info)->backing_image);
+ } else {
+ break;
+ }
+ }
}
- return info;
+
+ *p_info = info;
+ return;
+
+ err:
+ qapi_free_BlockInfo(info);
}
SnapshotInfoList *qmp_query_snapshots(Error **errp)
@@ -305,14 +330,23 @@ BlockInfoList *qmp_query_block(Error **errp)
{
BlockInfoList *head = NULL, **p_next = &head;
BlockDriverState *bs = NULL;
+ Error *local_err = NULL;
while ((bs = bdrv_next(bs))) {
BlockInfoList *info = g_malloc0(sizeof(*info));
- info->value = bdrv_query_info(bs);
+ bdrv_query_info(bs, &info->value, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ goto err;
+ }
*p_next = info;
p_next = &info->next;
}
return head;
+
+ err:
+ qapi_free_BlockInfoList(head);
+ return NULL;
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 0039a70..e0fd0a5 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -35,5 +35,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
int bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
Error **errp);
-BlockInfo *bdrv_query_info(BlockDriverState *bs);
+void bdrv_query_info(BlockDriverState *bs,
+ BlockInfo **p_info,
+ Error **errp);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 955ff05..68a3950 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -756,6 +756,8 @@
#
# @iops_wr: write I/O operations per second is specified
#
+# @image: the info of image used (since: 1.5)
+#
# Since: 0.14.0
#
# Notes: This interface is only found in @BlockInfo.
@@ -765,7 +767,8 @@
'*backing_file': 'str', 'backing_file_depth': 'int',
'encrypted': 'bool', 'encryption_key_missing': 'bool',
'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
- 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
+ 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+ 'image': 'ImageInfo' } }
##
# @BlockDeviceIoStatus:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e13c029..8db040a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1703,6 +1703,47 @@ Each json-object contain the following:
- "iops": limit total I/O operations per second (json-int)
- "iops_rd": limit read operations per second (json-int)
- "iops_wr": limit write operations per second (json-int)
+ - "image": the detail of the image, it is a json-object containing
+ the following:
+ - "filename": image file name (json-string)
+ - "format": image format (json-string)
+ - "virtual-size": image capacity in bytes (json-int)
+ - "dirty-flag": true if image is not cleanly closed, not present
+ means clean (json-bool, optional)
+ - "actual-size": actual size on disk in bytes of the image, not
+ present when image does not support thin
+ provision (json-int, optional)
+ - "cluster-size": size of a cluster in bytes, not present if image
+ format does not support it (json-int, optional)
+ - "encrypted": true if the image is encrypted, not present means
+ false or the image format does not support
+ encryption (json-bool, optional)
+ - "backing_file": backing file name, not present means no backing
+ file is used or the image format does not
+ support backing file chain
+ (json-string, optional)
+ - "full-backing-filename": full path of the backing file, not
+ present if it equals backing_file or no
+ backing file is used
+ (json-string, optional)
+ - "backing-filename-format": the format of the backing file, not
+ present means unknown or no backing
+ file (json-string, optional)
+ - "snapshots": the internal snapshot info, it is an optional list
+ of json-object containing the following:
+ - "id": unique snapshot id (json-string)
+ - "name": snapshot name (json-string)
+ - "vm-state-size": size of the VM state in bytes (json-int)
+ - "date-sec": UTC date of the snapshot in seconds (json-int)
+ - "date-nsec": fractional part in nanoseconds to be used with
+ date-sec(json-int)
+ - "vm-clock-sec": VM clock relative to boot in seconds
+ (json-int)
+ - "vm-clock-nsec": fractional part in nanoseconds to be used
+ with vm-clock-sec (json-int)
+ - "backing-image": the detail of the backing image, it is an
+ optional json-object only present when a
+ backing image present for this image
- "io-status": I/O operation status, only present if the device supports it
and the VM is configured to stop on errors. It's always reset
@@ -1723,14 +1764,38 @@ Example:
"ro":false,
"drv":"qcow2",
"encrypted":false,
- "file":"disks/test.img",
- "backing_file_depth":0,
+ "file":"disks/test.qcow2",
+ "backing_file_depth":1,
"bps":1000000,
"bps_rd":0,
"bps_wr":0,
"iops":1000000,
"iops_rd":0,
"iops_wr":0,
+ "image":{
+ "filename":"disks/test.qcow2",
+ "format":"qcow2",
+ "virtual-size":2048000,
+ "backing_file":"base.qcow2",
+ "full-backing-filename":"disks/base.qcow2",
+ "backing-filename-format:"qcow2",
+ "snapshots":[
+ {
+ "id": "1",
+ "name": "snapshot1",
+ "vm-state-size": 0,
+ "date-sec": 10000200,
+ "date-nsec": 12,
+ "vm-clock-sec": 206,
+ "vm-clock-nsec": 30
+ }
+ ],
+ "backing-image":{
+ "filename":"disks/base.qcow2",
+ "format":"qcow2",
+ "virtual-size":2048000
+ }
+ }
},
"type":"unknown"
},
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 13/18] block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (11 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 12/18] qmp: add ImageInfo in BlockDeviceInfo used by query-block Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump() Wenchao Xia
` (5 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
They are needed later in hmp command, dump_human_image_info()
is renamed to bdrv_image_info_dump().
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 33 ----------------
block/qapi.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 -
include/block/qapi.h | 2 +
qemu-img.c | 69 +---------------------------------
savevm.c | 1 +
6 files changed, 104 insertions(+), 102 deletions(-)
diff --git a/block.c b/block.c
index 126cd4b..1f05f7e 100644
--- a/block.c
+++ b/block.c
@@ -3430,39 +3430,6 @@ char *get_human_readable_size(char *buf, int buf_size, int64_t size)
return buf;
}
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
-{
- char buf1[128], date_buf[128], clock_buf[128];
- struct tm tm;
- time_t ti;
- int64_t secs;
-
- if (!sn) {
- snprintf(buf, buf_size,
- "%-10s%-20s%7s%20s%15s",
- "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
- } else {
- ti = sn->date_sec;
- localtime_r(&ti, &tm);
- strftime(date_buf, sizeof(date_buf),
- "%Y-%m-%d %H:%M:%S", &tm);
- secs = sn->vm_clock_nsec / 1000000000;
- snprintf(clock_buf, sizeof(clock_buf),
- "%02d:%02d:%02d.%03d",
- (int)(secs / 3600),
- (int)((secs / 60) % 60),
- (int)(secs % 60),
- (int)((sn->vm_clock_nsec / 1000000) % 1000));
- snprintf(buf, buf_size,
- "%-10s%-20s%7s%20s%15s",
- sn->id_str, sn->name,
- get_human_readable_size(buf1, sizeof(buf1), sn->vm_state_size),
- date_buf,
- clock_buf);
- }
- return buf;
-}
-
/**************************************************************/
/* async I/Os */
diff --git a/block/qapi.c b/block/qapi.c
index 24386b4..dc72adc 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -350,3 +350,103 @@ BlockInfoList *qmp_query_block(Error **errp)
qapi_free_BlockInfoList(head);
return NULL;
}
+
+char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+{
+ char buf1[128], date_buf[128], clock_buf[128];
+ struct tm tm;
+ time_t ti;
+ int64_t secs;
+
+ if (!sn) {
+ snprintf(buf, buf_size,
+ "%-10s%-20s%7s%20s%15s",
+ "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+ } else {
+ ti = sn->date_sec;
+ localtime_r(&ti, &tm);
+ strftime(date_buf, sizeof(date_buf),
+ "%Y-%m-%d %H:%M:%S", &tm);
+ secs = sn->vm_clock_nsec / 1000000000;
+ snprintf(clock_buf, sizeof(clock_buf),
+ "%02d:%02d:%02d.%03d",
+ (int)(secs / 3600),
+ (int)((secs / 60) % 60),
+ (int)(secs % 60),
+ (int)((sn->vm_clock_nsec / 1000000) % 1000));
+ snprintf(buf, buf_size,
+ "%-10s%-20s%7s%20s%15s",
+ sn->id_str, sn->name,
+ get_human_readable_size(buf1, sizeof(buf1), sn->vm_state_size),
+ date_buf,
+ clock_buf);
+ }
+ return buf;
+}
+
+void bdrv_image_info_dump(ImageInfo *info)
+{
+ char size_buf[128], dsize_buf[128];
+ if (!info->has_actual_size) {
+ snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
+ } else {
+ get_human_readable_size(dsize_buf, sizeof(dsize_buf),
+ info->actual_size);
+ }
+ get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
+ printf("image: %s\n"
+ "file format: %s\n"
+ "virtual size: %s (%" PRId64 " bytes)\n"
+ "disk size: %s\n",
+ info->filename, info->format, size_buf,
+ info->virtual_size,
+ dsize_buf);
+
+ if (info->has_encrypted && info->encrypted) {
+ printf("encrypted: yes\n");
+ }
+
+ if (info->has_cluster_size) {
+ printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+ }
+
+ if (info->has_dirty_flag && info->dirty_flag) {
+ printf("cleanly shut down: no\n");
+ }
+
+ if (info->has_backing_filename) {
+ printf("backing file: %s", info->backing_filename);
+ if (info->has_full_backing_filename) {
+ printf(" (actual path: %s)", info->full_backing_filename);
+ }
+ putchar('\n');
+ if (info->has_backing_filename_format) {
+ printf("backing file format: %s\n", info->backing_filename_format);
+ }
+ }
+
+ if (info->has_snapshots) {
+ SnapshotInfoList *elem;
+ char buf[256];
+
+ printf("Snapshot list:\n");
+ printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+ /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
+ * we convert to the block layer's native QEMUSnapshotInfo for now.
+ */
+ for (elem = info->snapshots; elem; elem = elem->next) {
+ QEMUSnapshotInfo sn = {
+ .vm_state_size = elem->value->vm_state_size,
+ .date_sec = elem->value->date_sec,
+ .date_nsec = elem->value->date_nsec,
+ .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
+ elem->value->vm_clock_nsec,
+ };
+
+ pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
+ pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
+ printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+ }
+ }
+}
diff --git a/include/block/block.h b/include/block/block.h
index a319f0a..ce28241 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -339,7 +339,6 @@ int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int bdrv_snapshot_load_tmp(BlockDriverState *bs,
const char *snapshot_name);
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
char *get_human_readable_size(char *buf, int buf_size, int64_t size);
int path_is_absolute(const char *path);
diff --git a/include/block/qapi.h b/include/block/qapi.h
index e0fd0a5..237660f 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -38,4 +38,6 @@ int bdrv_query_image_info(BlockDriverState *bs,
void bdrv_query_info(BlockDriverState *bs,
BlockInfo **p_info,
Error **errp);
+char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
+void bdrv_image_info_dump(ImageInfo *info);
#endif
diff --git a/qemu-img.c b/qemu-img.c
index 1dd0a60..5b229a9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1606,73 +1606,6 @@ static void dump_json_image_info(ImageInfo *info)
QDECREF(str);
}
-static void dump_human_image_info(ImageInfo *info)
-{
- char size_buf[128], dsize_buf[128];
- if (!info->has_actual_size) {
- snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
- } else {
- get_human_readable_size(dsize_buf, sizeof(dsize_buf),
- info->actual_size);
- }
- get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
- printf("image: %s\n"
- "file format: %s\n"
- "virtual size: %s (%" PRId64 " bytes)\n"
- "disk size: %s\n",
- info->filename, info->format, size_buf,
- info->virtual_size,
- dsize_buf);
-
- if (info->has_encrypted && info->encrypted) {
- printf("encrypted: yes\n");
- }
-
- if (info->has_cluster_size) {
- printf("cluster_size: %" PRId64 "\n", info->cluster_size);
- }
-
- if (info->has_dirty_flag && info->dirty_flag) {
- printf("cleanly shut down: no\n");
- }
-
- if (info->has_backing_filename) {
- printf("backing file: %s", info->backing_filename);
- if (info->has_full_backing_filename) {
- printf(" (actual path: %s)", info->full_backing_filename);
- }
- putchar('\n');
- if (info->has_backing_filename_format) {
- printf("backing file format: %s\n", info->backing_filename_format);
- }
- }
-
- if (info->has_snapshots) {
- SnapshotInfoList *elem;
- char buf[256];
-
- printf("Snapshot list:\n");
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
-
- /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
- * we convert to the block layer's native QEMUSnapshotInfo for now.
- */
- for (elem = info->snapshots; elem; elem = elem->next) {
- QEMUSnapshotInfo sn = {
- .vm_state_size = elem->value->vm_state_size,
- .date_sec = elem->value->date_sec,
- .date_nsec = elem->value->date_nsec,
- .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
- elem->value->vm_clock_nsec,
- };
-
- pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
- pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
- }
- }
-}
-
static void dump_human_image_info_list(ImageInfoList *list)
{
ImageInfoList *elem;
@@ -1684,7 +1617,7 @@ static void dump_human_image_info_list(ImageInfoList *list)
}
delim = true;
- dump_human_image_info(elem->value);
+ bdrv_image_info_dump(elem->value);
}
}
diff --git a/savevm.c b/savevm.c
index ed6d74c..1c8fbee 100644
--- a/savevm.c
+++ b/savevm.c
@@ -41,6 +41,7 @@
#include "qemu/bitops.h"
#include "qemu/iov.h"
#include "block/snapshot.h"
+#include "block/qapi.h"
#define SELF_ANNOUNCE_ROUNDS 5
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (12 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 13/18] block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-16 23:42 ` Eric Blake
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 15/18] hmp: add function hmp_info_snapshots() Wenchao Xia
` (4 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch introduce a new print function, which will output message to
monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
and can avoid string truncation, bdrv_image_info_dump() can also be used by
hmp code later, besides qemu-img code.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qapi.c | 58 +++++++++++++++++++++---------------------
include/block/qapi.h | 2 +-
include/qemu/error-report.h | 1 +
qemu-img.c | 7 +++--
savevm.c | 7 +++--
util/qemu-error.c | 18 +++++++++++++
6 files changed, 57 insertions(+), 36 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index dc72adc..6e04a7f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -351,7 +351,7 @@ BlockInfoList *qmp_query_block(Error **errp)
return NULL;
}
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
{
char buf1[128], date_buf[128], clock_buf[128];
struct tm tm;
@@ -359,9 +359,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
int64_t secs;
if (!sn) {
- snprintf(buf, buf_size,
- "%-10s%-20s%7s%20s%15s",
- "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+ message_printf("%-10s%-20s%7s%20s%15s",
+ "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
} else {
ti = sn->date_sec;
localtime_r(&ti, &tm);
@@ -374,14 +373,13 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
(int)((secs / 60) % 60),
(int)(secs % 60),
(int)((sn->vm_clock_nsec / 1000000) % 1000));
- snprintf(buf, buf_size,
- "%-10s%-20s%7s%20s%15s",
- sn->id_str, sn->name,
- get_human_readable_size(buf1, sizeof(buf1), sn->vm_state_size),
- date_buf,
- clock_buf);
+ message_printf("%-10s%-20s%7s%20s%15s",
+ sn->id_str, sn->name,
+ get_human_readable_size(buf1, sizeof(buf1),
+ sn->vm_state_size),
+ date_buf,
+ clock_buf);
}
- return buf;
}
void bdrv_image_info_dump(ImageInfo *info)
@@ -394,43 +392,44 @@ void bdrv_image_info_dump(ImageInfo *info)
info->actual_size);
}
get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
- printf("image: %s\n"
- "file format: %s\n"
- "virtual size: %s (%" PRId64 " bytes)\n"
- "disk size: %s\n",
- info->filename, info->format, size_buf,
- info->virtual_size,
- dsize_buf);
+ message_printf("image: %s\n"
+ "file format: %s\n"
+ "virtual size: %s (%" PRId64 " bytes)\n"
+ "disk size: %s\n",
+ info->filename, info->format, size_buf,
+ info->virtual_size,
+ dsize_buf);
if (info->has_encrypted && info->encrypted) {
- printf("encrypted: yes\n");
+ message_printf("encrypted: yes\n");
}
if (info->has_cluster_size) {
- printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+ message_printf("cluster_size: %" PRId64 "\n", info->cluster_size);
}
if (info->has_dirty_flag && info->dirty_flag) {
- printf("cleanly shut down: no\n");
+ message_printf("cleanly shut down: no\n");
}
if (info->has_backing_filename) {
- printf("backing file: %s", info->backing_filename);
+ message_printf("backing file: %s", info->backing_filename);
if (info->has_full_backing_filename) {
- printf(" (actual path: %s)", info->full_backing_filename);
+ message_printf(" (actual path: %s)", info->full_backing_filename);
}
- putchar('\n');
+ message_printf("\n");
if (info->has_backing_filename_format) {
- printf("backing file format: %s\n", info->backing_filename_format);
+ message_printf("backing file format: %s\n",
+ info->backing_filename_format);
}
}
if (info->has_snapshots) {
SnapshotInfoList *elem;
- char buf[256];
- printf("Snapshot list:\n");
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ message_printf("Snapshot list:\n");
+ bdrv_snapshot_dump(NULL);
+ message_printf("\n");
/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
* we convert to the block layer's native QEMUSnapshotInfo for now.
@@ -446,7 +445,8 @@ void bdrv_image_info_dump(ImageInfo *info)
pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+ bdrv_snapshot_dump(&sn);
+ message_printf("\n");
}
}
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 237660f..f7eda89 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -38,6 +38,6 @@ int bdrv_query_image_info(BlockDriverState *bs,
void bdrv_query_info(BlockDriverState *bs,
BlockInfo **p_info,
Error **errp);
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
+void bdrv_snapshot_dump(QEMUSnapshotInfo *sn);
void bdrv_image_info_dump(ImageInfo *info);
#endif
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index c902cc1..90341bd 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -40,4 +40,5 @@ void error_set_progname(const char *argv0);
void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
const char *error_get_progname(void);
+void message_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
#endif
diff --git a/qemu-img.c b/qemu-img.c
index 5b229a9..8cbcb0b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1558,16 +1558,17 @@ static void dump_snapshots(BlockDriverState *bs)
{
QEMUSnapshotInfo *sn_tab, *sn;
int nb_sns, i;
- char buf[256];
nb_sns = bdrv_snapshot_list(bs, &sn_tab);
if (nb_sns <= 0)
return;
printf("Snapshot list:\n");
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ bdrv_snapshot_dump(NULL);
+ printf("\n");
for(i = 0; i < nb_sns; i++) {
sn = &sn_tab[i];
- printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+ bdrv_snapshot_dump(sn);
+ printf("\n");
}
g_free(sn_tab);
}
diff --git a/savevm.c b/savevm.c
index 1c8fbee..e125567 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2466,7 +2466,6 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
int nb_sns, i, ret, available;
int total;
int *available_snapshots;
- char buf[256];
bs = bdrv_snapshots();
if (!bs) {
@@ -2509,10 +2508,12 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
}
if (total > 0) {
- monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ bdrv_snapshot_dump(NULL);
+ monitor_printf(mon, "\n");
for (i = 0; i < total; i++) {
sn = &sn_tab[available_snapshots[i]];
- monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+ bdrv_snapshot_dump(sn);
+ monitor_printf(mon, "\n");
}
} else {
monitor_printf(mon, "There is no suitable snapshot available\n");
diff --git a/util/qemu-error.c b/util/qemu-error.c
index 08a36f4..a47bf32 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -213,3 +213,21 @@ void error_report(const char *fmt, ...)
va_end(ap);
error_printf("\n");
}
+
+/*
+ * Print to current monitor if we have one, else to stdout. It is similar with
+ * error_printf().
+ * TODO just like error_vprintf()
+ */
+void message_printf(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ if (cur_mon) {
+ monitor_vprintf(cur_mon, fmt, ap);
+ } else {
+ vfprintf(stdout, fmt, ap);
+ }
+ va_end(ap);
+}
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump() Wenchao Xia
@ 2013-04-16 23:42 ` Eric Blake
2013-04-17 5:25 ` Wenchao Xia
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2013-04-16 23:42 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, armbru, qemu-devel, lcapitulino, stefanha,
pbonzini
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
> This patch introduce a new print function, which will output message to
> monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
> and can avoid string truncation, bdrv_image_info_dump() can also be used by
> hmp code later, besides qemu-img code.
Not a full review (yet), but can we get this particular cleanup rebased
to be done first in isolation, since Pavel's patches would benefit from
being based on top of this cleanup as well?
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03287.html
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
2013-04-16 23:42 ` Eric Blake
@ 2013-04-17 5:25 ` Wenchao Xia
0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-17 5:25 UTC (permalink / raw)
To: Eric Blake
Cc: kwolf, phrdina, qemu-devel, stefanha, armbru, lcapitulino,
pbonzini
于 2013-4-17 7:42, Eric Blake 写道:
> On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>> This patch introduce a new print function, which will output message to
>> monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
>> and can avoid string truncation, bdrv_image_info_dump() can also be used by
>> hmp code later, besides qemu-img code.
>
> Not a full review (yet), but can we get this particular cleanup rebased
> to be done first in isolation, since Pavel's patches would benefit from
> being based on top of this cleanup as well?
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03287.html
>
I hope this serial have no more issues to be commited, avoiding
troubles in rebasing and file moves.
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH V12 15/18] hmp: add function hmp_info_snapshots()
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (13 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 16/18] hmp: switch snapshot info function to qmp based one Wenchao Xia
` (3 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This function will simply call qmp interface qmp_query_snapshots()
added in last commit and then dump information in monitor console.
To get snapshot info, Now qemu and qemu-img both call block layer
function bdrv_query_snapshot_info_list() in their calling path, and
then they just translate the qmp object got to strings in stdout or
monitor console.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
hmp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 1 +
2 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/hmp.c b/hmp.c
index dbe9b90..cef5eee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -22,6 +22,7 @@
#include "qemu/sockets.h"
#include "monitor/monitor.h"
#include "ui/console.h"
+#include "block/qapi.h"
static void hmp_handle_error(Monitor *mon, Error **errp)
{
@@ -653,6 +654,49 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
qapi_free_TPMInfoList(info_list);
}
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
+{
+ SnapshotInfoList *elem;
+
+ bdrv_snapshot_dump(NULL);
+ monitor_printf(mon, "\n");
+
+ for (elem = list; elem; elem = elem->next) {
+ QEMUSnapshotInfo sn = {
+ .vm_state_size = elem->value->vm_state_size,
+ .date_sec = elem->value->date_sec,
+ .date_nsec = elem->value->date_nsec,
+ .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
+ elem->value->vm_clock_nsec,
+ };
+ pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
+ pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
+ bdrv_snapshot_dump(&sn);
+ monitor_printf(mon, "\n");
+ }
+}
+
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ SnapshotInfoList *list;
+
+ list = qmp_query_snapshots(&err);
+ if (error_is_set(&err)) {
+ hmp_handle_error(mon, &err);
+ return;
+ }
+
+ if (list == NULL) {
+ monitor_printf(mon, "There is no suitable snapshot available\n");
+ return;
+ }
+
+ monitor_dump_snapshotinfolist(mon, list);
+ qapi_free_SnapshotInfoList(list);
+}
+
void hmp_quit(Monitor *mon, const QDict *qdict)
{
monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 80e8b41..1172c47 100644
--- a/hmp.h
+++ b/hmp.h
@@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
void hmp_info_pci(Monitor *mon, const QDict *qdict);
void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
void hmp_quit(Monitor *mon, const QDict *qdict);
void hmp_stop(Monitor *mon, const QDict *qdict);
void hmp_system_reset(Monitor *mon, const QDict *qdict);
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 16/18] hmp: switch snapshot info function to qmp based one
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (14 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 15/18] hmp: add function hmp_info_snapshots() Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 17/18] hmp: show ImageInfo in 'info block' Wenchao Xia
` (2 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
This patch using new added function in last commit which retrieve
info from qmp for snapshot info.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
monitor.c | 2 +-
savevm.c | 65 -------------------------------------------------------------
2 files changed, 1 insertions(+), 66 deletions(-)
diff --git a/monitor.c b/monitor.c
index c897e80..a2b1e3f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2630,7 +2630,7 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the currently saved VM snapshots",
- .mhandler.cmd = do_info_snapshots,
+ .mhandler.cmd = hmp_info_snapshots,
},
{
.name = "status",
diff --git a/savevm.c b/savevm.c
index e125567..59e329e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2459,71 +2459,6 @@ void do_delvm(Monitor *mon, const QDict *qdict)
}
}
-void do_info_snapshots(Monitor *mon, const QDict *qdict)
-{
- BlockDriverState *bs, *bs1;
- QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
- int nb_sns, i, ret, available;
- int total;
- int *available_snapshots;
-
- bs = bdrv_snapshots();
- if (!bs) {
- monitor_printf(mon, "No available block device supports snapshots\n");
- return;
- }
-
- nb_sns = bdrv_snapshot_list(bs, &sn_tab);
- if (nb_sns < 0) {
- monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
- return;
- }
-
- if (nb_sns == 0) {
- monitor_printf(mon, "There is no snapshot available.\n");
- return;
- }
-
- available_snapshots = g_malloc0(sizeof(int) * nb_sns);
- total = 0;
- for (i = 0; i < nb_sns; i++) {
- sn = &sn_tab[i];
- available = 1;
- bs1 = NULL;
-
- while ((bs1 = bdrv_next(bs1))) {
- if (bdrv_can_snapshot(bs1) && bs1 != bs) {
- ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
- if (ret < 0) {
- available = 0;
- break;
- }
- }
- }
-
- if (available) {
- available_snapshots[total] = i;
- total++;
- }
- }
-
- if (total > 0) {
- bdrv_snapshot_dump(NULL);
- monitor_printf(mon, "\n");
- for (i = 0; i < total; i++) {
- sn = &sn_tab[available_snapshots[i]];
- bdrv_snapshot_dump(sn);
- monitor_printf(mon, "\n");
- }
- } else {
- monitor_printf(mon, "There is no suitable snapshot available\n");
- }
-
- g_free(sn_tab);
- g_free(available_snapshots);
-
-}
-
void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
{
qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 17/18] hmp: show ImageInfo in 'info block'
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (15 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 16/18] hmp: switch snapshot info function to qmp based one Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 18/18] hmp: add parameters device and -v for info block Wenchao Xia
2013-04-22 2:58 ` [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
Now human monitor can show image details, include internal
snapshot and backing chain info for every block device.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
hmp.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hmp.c b/hmp.c
index cef5eee..1d13030 100644
--- a/hmp.c
+++ b/hmp.c
@@ -278,6 +278,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
void hmp_info_block(Monitor *mon, const QDict *qdict)
{
BlockInfoList *block_list, *info;
+ ImageInfo *image_info;
block_list = qmp_query_block(NULL);
@@ -319,6 +320,17 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
info->value->inserted->iops,
info->value->inserted->iops_rd,
info->value->inserted->iops_wr);
+
+ monitor_printf(mon, " images:\n");
+ image_info = info->value->inserted->image;
+ while (1) {
+ bdrv_image_info_dump(image_info);
+ if (image_info->has_backing_image) {
+ image_info = image_info->backing_image;
+ } else {
+ break;
+ }
+ }
} else {
monitor_printf(mon, " [not inserted]");
}
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH V12 18/18] hmp: add parameters device and -v for info block
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (16 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 17/18] hmp: show ImageInfo in 'info block' Wenchao Xia
@ 2013-04-13 8:56 ` Wenchao Xia
2013-04-22 2:58 ` [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
18 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-04-13 8:56 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, phrdina, stefanha, armbru, lcapitulino, pbonzini,
Wenchao Xia
With these parameters, user can choose the information to be showed,
to avoid message flood in the monitor.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
hmp.c | 23 +++++++++++++++--------
monitor.c | 7 ++++---
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/hmp.c b/hmp.c
index 1d13030..4780873 100644
--- a/hmp.c
+++ b/hmp.c
@@ -279,10 +279,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
{
BlockInfoList *block_list, *info;
ImageInfo *image_info;
+ const char *device = qdict_get_try_str(qdict, "device");
+ int verbose = qdict_get_try_bool(qdict, "verbose", 0);
block_list = qmp_query_block(NULL);
for (info = block_list; info; info = info->next) {
+ if (device && strcmp(device, info->value->device)) {
+ continue;
+ }
monitor_printf(mon, "%s: removable=%d",
info->value->device, info->value->removable);
@@ -321,14 +326,16 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
info->value->inserted->iops_rd,
info->value->inserted->iops_wr);
- monitor_printf(mon, " images:\n");
- image_info = info->value->inserted->image;
- while (1) {
- bdrv_image_info_dump(image_info);
- if (image_info->has_backing_image) {
- image_info = image_info->backing_image;
- } else {
- break;
+ if (verbose) {
+ monitor_printf(mon, " images:\n");
+ image_info = info->value->inserted->image;
+ while (1) {
+ bdrv_image_info_dump(image_info);
+ if (image_info->has_backing_image) {
+ image_info = image_info->backing_image;
+ } else {
+ break;
+ }
}
}
} else {
diff --git a/monitor.c b/monitor.c
index a2b1e3f..91a7066 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2474,9 +2474,10 @@ static mon_cmd_t info_cmds[] = {
},
{
.name = "block",
- .args_type = "",
- .params = "",
- .help = "show the block devices",
+ .args_type = "verbose:-v,device:B?",
+ .params = "[-v] [device]",
+ .help = "show info of one block device or all block devices "
+ "(and details of images with -v option)",
.mhandler.cmd = hmp_info_block,
},
{
--
1.7.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info
2013-04-13 8:56 [Qemu-devel] [PATCH V12 00/18] qmp/hmp interfaces for internal snapshot info Wenchao Xia
` (17 preceding siblings ...)
2013-04-13 8:56 ` [Qemu-devel] [PATCH V12 18/18] hmp: add parameters device and -v for info block Wenchao Xia
@ 2013-04-22 2:58 ` Wenchao Xia
2013-04-22 8:27 ` Markus Armbruster
18 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-04-22 2:58 UTC (permalink / raw)
To: Wenchao Xia
Cc: kwolf, phrdina, stefanha, qemu-devel, armbru, pbonzini,
lcapitulino
Hi,
Any other comments for it, especially HMP part?
>
> V12:
> Address Markus's comments:
> 02/18: better incode comments for bdrv_snapshot_find(), add tip about the
> logic change in snapshot create/delete/load/info if some snapshot's id is mixed
> with name in commit message.
> 05/18: better incode comments for bdrv_query_snapshot_info_list(), do not
> check return value in caller qemu-img.
> 07/18: new patch changing the vm snapshot filter logic, to be exactly the
> same with load_vmstate(). To tip it clearly this patch is made a seperate one,
> and can be droped if original logic want to be kepted.
> 08/18: squash -ENOMEDIUM and -ENOTSUP in switch statement in
> bdrv_query_image_info().
> 11/18: better incode comments for bdrv_query_image_info().
> 12/18: change disk name suffix from .img to .qcow2 in example, to tip better.
> 14/18: new funtion message_printf(), which automatically dump to monitor if
> it present, discard buffer or GString.
> 15/18: use message_printf() instead of buffer.
> 17/18: use message_printf() instead of buffer.
>
> Address Eric's comments:
> 05/18: better incode comments for bdrv_query_snapshot_info_list(), do not
> check return value in caller qemu-img.
>
> Address Stefan's comments:
> 10/18: added doc about the meaning of "consistent snapshot" in
> qmp-commands.hx.
>
> Address Kevin's comments:
> 12/18: use local_error to detect error in qmp_query_snapshots(), in case
> caller set errp = NULL.
>
> Wenchao Xia (18):
> 1 block: move bdrv_snapshot_find() to block/snapshot.c
> 2 block: distinguish id and name in bdrv_find_snapshot()
> 3 qemu-img: remove unused parameter in collect_image_info()
> 4 block: move collect_snapshots() and collect_image_info() to block/qapi.c
> 5 block: add snapshot info query function bdrv_query_snapshot_info_list()
> 6 block: add check for VM snapshot in bdrv_query_snapshot_info_list()
> 7 block: change VM snapshot checking logic
> 8 block: add image info query function bdrv_query_image_info()
> 9 block: move qmp_query_block() and bdrv_query_info() to block/qapi.c
> 10 qmp: add interface query-snapshots
> 11 qmp: add recursive member in ImageInfo
> 12 qmp: add ImageInfo in BlockDeviceInfo used by query-block
> 13 block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c
> 14 block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
> 15 hmp: add function hmp_info_snapshots()
> 16 hmp: switch snapshot info function to qmp based one
> 17 hmp: show ImageInfo in 'info block'
> 18 hmp: add parameters device and -v for info block
>
> block.c | 109 -----------
> block/Makefile.objs | 1 +
> block/qapi.c | 452 +++++++++++++++++++++++++++++++++++++++++++
> block/snapshot.c | 78 ++++++++
> hmp.c | 63 ++++++
> hmp.h | 1 +
> include/block/block.h | 2 -
> include/block/qapi.h | 43 ++++
> include/block/snapshot.h | 37 ++++
> include/qemu/error-report.h | 1 +
> monitor.c | 9 +-
> qapi-schema.json | 24 ++-
> qemu-img.c | 165 +---------------
> qmp-commands.hx | 127 ++++++++++++-
> savevm.c | 96 +---------
> util/qemu-error.c | 18 ++
> 16 files changed, 861 insertions(+), 365 deletions(-)
> create mode 100644 block/qapi.c
> create mode 100644 block/snapshot.c
> create mode 100644 include/block/qapi.h
> create mode 100644 include/block/snapshot.h
>
>
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 30+ messages in thread