| Message ID | 1276653214-15427-1-git-send-email-miguel.filho@gmail.com |
|---|---|
| State | New |
| Headers | show |
Am 16.06.2010 03:53, schrieb Miguel Di Ciurcio Filho: > The 'info snapshots' monitor command does not show snapshot information from all > available block devices. > > Usage example: > $ qemu -hda disk1.qcow2 -hdb disk2.qcow2 > > (qemu) info snapshots > Snapshot devices: ide0-hd0 > Snapshot list (from ide0-hd0): > ID TAG VM SIZE DATE VM CLOCK > 1 1.5M 2010-05-26 21:51:02 00:00:03.263 > 2 1.5M 2010-05-26 21:51:09 00:00:08.844 > 3 1.5M 2010-05-26 21:51:24 00:00:23.274 > 4 1.5M 2010-05-26 21:53:17 00:00:03.595 > > In the above case, disk2.qcow2 has snapshot information, but it is not being > shown. Only the first device is always shown. > > This patch updates the do_info_snapshots() function do correctly show snapshot > information about all available block devices. > > New output: > (qemu) info snapshots > Snapshot list from ide0-hd0: > ID TAG VM SIZE DATE VM CLOCK > 1 1.5M 2010-05-26 21:51:02 00:00:03.263 > 2 1.5M 2010-05-26 21:51:09 00:00:08.844 > 3 1.5M 2010-05-26 21:51:24 00:00:23.274 > 4 1.5M 2010-05-26 21:53:17 00:00:03.595 > > Snapshot list from ide0-hd1: > ID TAG VM SIZE DATE VM CLOCK > 1 0 2010-05-26 21:51:02 00:00:03.263 > 2 0 2010-05-26 21:51:09 00:00:08.844 > 3 0 2010-05-26 21:51:24 00:00:23.274 > 4 0 2010-05-26 21:53:17 00:00:03.595 > > Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com> If the human monitor was exactly what its name says, I'd happily apply this one (though I think it should be made clear from which image the VM state would be loaded). However, it isn't and I'm not sure if this wouldn't break libvirt. Dan, can you help? If we can't change the info snapshots output, a possible alternative would be to introduce another info command for this. Kevin
On Wed, Jun 16, 2010 at 9:40 AM, Kevin Wolf <kwolf@redhat.com> wrote: > > If the human monitor was exactly what its name says, I'd happily apply > this one (though I think it should be made clear from which image the VM > state would be loaded). However, it isn't and I'm not sure if this > wouldn't break libvirt. Dan, can you help? > I didn't mention in the commit, but I've looked at libvirt's source and it is not using 'info snapshots' AFAIK. At the present time, the VM state is always saved in the first block device that supports snapshots. I could update the patch to make it clear on the output somehow. From savevm.c:get_bs_snapshot(): static BlockDriverState *get_bs_snapshots(void) { BlockDriverState *bs; if (bs_snapshots) return bs_snapshots; /* FIXME what if bs_snapshots gets hot-unplugged? */ bs = NULL; while ((bs = bdrv_next(bs))) { if (bdrv_can_snapshot(bs)) { goto ok; } } return NULL; ok: bs_snapshots = bs; return bs; } Regards, Miguel
Am 16.06.2010 14:59, schrieb Miguel Di Ciurcio Filho: > On Wed, Jun 16, 2010 at 9:40 AM, Kevin Wolf <kwolf@redhat.com> wrote: >> >> If the human monitor was exactly what its name says, I'd happily apply >> this one (though I think it should be made clear from which image the VM >> state would be loaded). However, it isn't and I'm not sure if this >> wouldn't break libvirt. Dan, can you help? >> > > I didn't mention in the commit, but I've looked at libvirt's source > and it is not using 'info snapshots' AFAIK. Anthony, Dan, are you okay with the change then? > At the present time, the VM state is always saved in the first block > device that supports snapshots. I could update the patch to make it > clear on the output somehow. Would be nice to make it clear. Something like this maybe: Snapshot list from ide0-hd0 (VM state image): [...] Kevin
Am 16.06.2010 17:22, schrieb Chris Lalancette: > On 06/16/10 - 03:15:11PM, Kevin Wolf wrote: >> Am 16.06.2010 14:59, schrieb Miguel Di Ciurcio Filho: >>> On Wed, Jun 16, 2010 at 9:40 AM, Kevin Wolf <kwolf@redhat.com> wrote: >>>> >>>> If the human monitor was exactly what its name says, I'd happily apply >>>> this one (though I think it should be made clear from which image the VM >>>> state would be loaded). However, it isn't and I'm not sure if this >>>> wouldn't break libvirt. Dan, can you help? >>>> >>> >>> I didn't mention in the commit, but I've looked at libvirt's source >>> and it is not using 'info snapshots' AFAIK. >> >> Anthony, Dan, are you okay with the change then? > > Right, exactly as Miguel said, libvirt doesn't use "info snapshots" at all > at the moment. One of the reasons we don't use it at present is precisely > because it doesn't give us information about all disks in-use. > > The other reason that we can't use "info snapshots" is that we need to know > parent information about snapshots. That is, if you take a sequence of > snapshots: > > A -> B -> C > > And then you delete B, the disk changes from B will be merged automatically > into C to keep C a valid snapshot. However, there is currently no way to > discover this parent/child relationship, so we can't use "info snapshots" > for that reason as well. Well, there is no parent/child relation in qcow2, so exposing this is going to be really hard. We also don't really need it anywhere in qemu. What would libvirt use this information for? Kevin
On Wed, Jun 16, 2010 at 12:22 PM, Chris Lalancette <clalance@redhat.com> wrote: >> > I didn't mention in the commit, but I've looked at libvirt's source >> > and it is not using 'info snapshots' AFAIK. >> >> Anthony, Dan, are you okay with the change then? > > Right, exactly as Miguel said, libvirt doesn't use "info snapshots" at all > at the moment. One of the reasons we don't use it at present is precisely > because it doesn't give us information about all disks in-use. > > The other reason that we can't use "info snapshots" is that we need to know > parent information about snapshots. That is, if you take a sequence of > snapshots: > So lets get there, we need to start from somewhere :-D >> >> > At the present time, the VM state is always saved in the first block >> > device that supports snapshots. I could update the patch to make it >> > clear on the output somehow. >> >> Would be nice to make it clear. Something like this maybe: >> >> Snapshot list from ide0-hd0 (VM state image): >> [...] > > Yes, I also agree this would be a good idea. When the QMP version of this > finally gets implemented, we'll want some way to distinguish where the > VM state is stored as well. > I will update the patch as proposed by Kevin and resend. Regards, Miguel
Am 16.06.2010 17:57, schrieb Chris Lalancette: > On 06/16/10 - 05:32:58PM, Kevin Wolf wrote: >> Am 16.06.2010 17:22, schrieb Chris Lalancette: >>> On 06/16/10 - 03:15:11PM, Kevin Wolf wrote: >>>> Am 16.06.2010 14:59, schrieb Miguel Di Ciurcio Filho: >>>>> On Wed, Jun 16, 2010 at 9:40 AM, Kevin Wolf <kwolf@redhat.com> wrote: >>>>>> >>>>>> If the human monitor was exactly what its name says, I'd happily apply >>>>>> this one (though I think it should be made clear from which image the VM >>>>>> state would be loaded). However, it isn't and I'm not sure if this >>>>>> wouldn't break libvirt. Dan, can you help? >>>>>> >>>>> >>>>> I didn't mention in the commit, but I've looked at libvirt's source >>>>> and it is not using 'info snapshots' AFAIK. >>>> >>>> Anthony, Dan, are you okay with the change then? >>> >>> Right, exactly as Miguel said, libvirt doesn't use "info snapshots" at all >>> at the moment. One of the reasons we don't use it at present is precisely >>> because it doesn't give us information about all disks in-use. >>> >>> The other reason that we can't use "info snapshots" is that we need to know >>> parent information about snapshots. That is, if you take a sequence of >>> snapshots: >>> >>> A -> B -> C >>> >>> And then you delete B, the disk changes from B will be merged automatically >>> into C to keep C a valid snapshot. However, there is currently no way to >>> discover this parent/child relationship, so we can't use "info snapshots" >>> for that reason as well. >> >> Well, there is no parent/child relation in qcow2, so exposing this is >> going to be really hard. We also don't really need it anywhere in qemu. >> What would libvirt use this information for? > > I keep being told this, and I don't really understand how this is. I know > when I was heavily playing with this, the scenario above worked; that is, the > deletion of snapshot B maintained a valid C snapshot. If nothing is tracking > the parent/child relationship, how does this work? Clusters are refcounted. When you save a snapshot, the refcount for all clusters in the current state is increased. When you delete it, the refcount is decreased and only if it's zero the cluster is freed. > As for how libvirt uses it, it's mostly to provide the ability for the user > to keep track of a "tree" of snapshots. So the user could do something like > install their base OS, and take a snapshot S1. Then they could install one set > of applications, and take a snapshot S2. Now they can go back to the base > image, install a different set of applications, and take a snapshot S3. > Now both S2 and S3 are children of S1, and libvirt wants to be able to > represent this relationship. qemu doesn't even remember which snapshot you have loaded. Basically you have one L1 table for active cluster allocations and you have another one for each snapshot. When you load a snapshot, it just copies the L1 table to the active one (and adjusts refcounts). So technically the concept of a snapshot tree doesn't exist with internal snapshots. It's something that management introduces. Kevin
diff --git a/savevm.c b/savevm.c index 20354a8..0eacb9f 100644 --- a/savevm.c +++ b/savevm.c @@ -1858,8 +1858,8 @@ void do_delvm(Monitor *mon, const QDict *qdict) void do_info_snapshots(Monitor *mon) { - BlockDriverState *bs, *bs1; - QEMUSnapshotInfo *sn_tab, *sn; + BlockDriverState *bs; + QEMUSnapshotInfo *sn_tab; int nb_sns, i; char buf[256]; @@ -1868,27 +1868,27 @@ void do_info_snapshots(Monitor *mon) monitor_printf(mon, "No available block device supports snapshots\n"); return; } - monitor_printf(mon, "Snapshot devices:"); - bs1 = NULL; - while ((bs1 = bdrv_next(bs1))) { - if (bdrv_can_snapshot(bs1)) { - if (bs == bs1) - monitor_printf(mon, " %s", bdrv_get_device_name(bs1)); - } - } - monitor_printf(mon, "\n"); - nb_sns = bdrv_snapshot_list(bs, &sn_tab); - if (nb_sns < 0) { - monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns); - return; - } - monitor_printf(mon, "Snapshot list (from %s):\n", - bdrv_get_device_name(bs)); - monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); - for(i = 0; i < nb_sns; i++) { - sn = &sn_tab[i]; - monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn)); + bs = NULL; + while ((bs = bdrv_next(bs))) { + if (bdrv_can_snapshot(bs)) { + monitor_printf(mon, "Snapshot list from %s:\n", + bdrv_get_device_name(bs)); + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); + + nb_sns = bdrv_snapshot_list(bs, &sn_tab); + if (nb_sns < 0) { + monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns); + continue; + } + + for (i = 0; i < nb_sns; i++) { + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), + &sn_tab[i])); + } + + qemu_free(sn_tab); + monitor_printf(mon, "\n"); + } } - qemu_free(sn_tab); }
The 'info snapshots' monitor command does not show snapshot information from all available block devices. Usage example: $ qemu -hda disk1.qcow2 -hdb disk2.qcow2 (qemu) info snapshots Snapshot devices: ide0-hd0 Snapshot list (from ide0-hd0): ID TAG VM SIZE DATE VM CLOCK 1 1.5M 2010-05-26 21:51:02 00:00:03.263 2 1.5M 2010-05-26 21:51:09 00:00:08.844 3 1.5M 2010-05-26 21:51:24 00:00:23.274 4 1.5M 2010-05-26 21:53:17 00:00:03.595 In the above case, disk2.qcow2 has snapshot information, but it is not being shown. Only the first device is always shown. This patch updates the do_info_snapshots() function do correctly show snapshot information about all available block devices. New output: (qemu) info snapshots Snapshot list from ide0-hd0: ID TAG VM SIZE DATE VM CLOCK 1 1.5M 2010-05-26 21:51:02 00:00:03.263 2 1.5M 2010-05-26 21:51:09 00:00:08.844 3 1.5M 2010-05-26 21:51:24 00:00:23.274 4 1.5M 2010-05-26 21:53:17 00:00:03.595 Snapshot list from ide0-hd1: ID TAG VM SIZE DATE VM CLOCK 1 0 2010-05-26 21:51:02 00:00:03.263 2 0 2010-05-26 21:51:09 00:00:08.844 3 0 2010-05-26 21:51:24 00:00:23.274 4 0 2010-05-26 21:53:17 00:00:03.595 Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com> --- savevm.c | 46 +++++++++++++++++++++++----------------------- 1 files changed, 23 insertions(+), 23 deletions(-)