Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Am 29.01.2013 17:24, schrieb Eric Blake: On 01/29/2013 07:27 AM, Benoît Canet wrote: base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ For that matter, I think it would be useful to have a quorum where the data is duplicated between different formats, such as: file1 [raw] --\ base2 [raw] --- snap2.qcow2 [qcow2] quorum base3 [qed] --- snap3.qed [qed] --/ In that case, you NEED to be able to pass in the protocol of each underlying device _as part of opening the quorum_, something like: quorum:2/3:file1[raw]:snap2.qcow2[qcow2]:snap3.qed[qed] or some such punctuation. And of course, that means you'd have to extend your escaping mechanism to allow escaping of whatever syntax you use for declaring a per-member format. Let me rephrase that: quorum really needs -blockdev. And quite possibly the data that can be passed to the snapshot QMP commands today isn't sufficient to describe what we want to describe. :-/ Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Am 28.01.2013 18:04, schrieb Benoît Canet:
Signed-off-by: Benoit Canet ben...@irqsave.net
---
blockdev.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 0ce45c5..b1f388b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -800,7 +800,8 @@ void qmp_transaction(BlockdevActionList *dev_list, Error
**errp)
/* We will manually add the backing_hd field to the bs later */
states-new_bs = bdrv_new();
ret = bdrv_open(states-new_bs, new_image_file,
-flags | BDRV_O_NO_BACKING, drv);
+flags | BDRV_O_NO_BACKING,
+path_has_protocol(new_image_file) ? NULL : drv);
if (ret != 0) {
error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
goto delete_and_fail;
Wait, what's happening here? I don't understand this patch and how it's
related to snapshotting non-file protocols (if this is even what you
mean). What is your exact scenario, what does the existing code do in
it, and how does this change improve it? An empty commit message is
definitely not appropriate for such a change.
In any case, using NULL as drv for bdrv_open() looks plain wrong.
Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Wait, what's happening here? I don't understand this patch and how it's related to snapshotting non-file protocols (if this is even what you mean). What is your exact scenario, what does the existing code do in it, and how does this change improve it? An empty commit message is definitely not appropriate for such a change. In any case, using NULL as drv for bdrv_open() looks plain wrong. When passing drv bdrv_open tries to open the url as qcow2 or another plain file format and fail. bdrv_file_open is not a better option because it won't return a BlockDriverState constructed in the same way as the old one. (raw as bs and quorum as -file) I agree that this patch is a hack and I am looking for a better way of doing it. Benoît
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Am 29.01.2013 14:07, schrieb Benoît Canet: Wait, what's happening here? I don't understand this patch and how it's related to snapshotting non-file protocols (if this is even what you mean). What is your exact scenario, what does the existing code do in it, and how does this change improve it? An empty commit message is definitely not appropriate for such a change. In any case, using NULL as drv for bdrv_open() looks plain wrong. When passing drv bdrv_open tries to open the url as qcow2 or another plain file format and fail. What exactly is a plain file format and what is the difference between it and other file formats? Anyway, this is not a problem description, let alone a description of your exact scenario. If qcow2 is specified as the format in the QMP command, why is it then wrong to try and open the file as qcow2? Why does it fail? What is the correct driver? bdrv_file_open is not a better option because it won't return a BlockDriverState constructed in the same way as the old one. (raw as bs and quorum as -file) I agree that this patch is a hack and I am looking for a better way of doing it. Let's talk about the problem first before discussing solutions. Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Le Tuesday 29 Jan 2013 à 14:25:42 (+0100), Kevin Wolf a écrit : Am 29.01.2013 14:07, schrieb Benoît Canet: Wait, what's happening here? I don't understand this patch and how it's related to snapshotting non-file protocols (if this is even what you mean). What is your exact scenario, what does the existing code do in it, and how does this change improve it? An empty commit message is definitely not appropriate for such a change. In any case, using NULL as drv for bdrv_open() looks plain wrong. When passing drv bdrv_open tries to open the url as qcow2 or another plain file format and fail. What exactly is a plain file format and what is the difference between it and other file formats? Quorum is a protocol which encapsulate n BlockDriverState. Anyway, this is not a problem description, let alone a description of your exact scenario. A scenario would be: A user want to create a new quorum snapshot made of n qcow2 snapshots. The user specify qcow2 as format so the new quorum files are created as qcow2. Then the code try to open the quorum file as a qcow2 file and it fail. If qcow2 is specified as the format in the QMP command, why is it then wrong to try and open the file as qcow2? Why does it fail? What is the correct driver? It's correct to format the individual quorum files as qcow2 so specifying qcow2 in the qmp command seems ok. But opening a quorum: url which agregate these qcow2 files will fail if drv is the qcow2 driver. Would passing the quorum protocol driver to bdrv_open be ok ? Benoît bdrv_file_open is not a better option because it won't return a BlockDriverState constructed in the same way as the old one. (raw as bs and quorum as -file) I agree that this patch is a hack and I am looking for a better way of doing it. Let's talk about the problem first before discussing solutions. Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Am 29.01.2013 14:45, schrieb Benoît Canet: Le Tuesday 29 Jan 2013 à 14:25:42 (+0100), Kevin Wolf a écrit : Am 29.01.2013 14:07, schrieb Benoît Canet: Wait, what's happening here? I don't understand this patch and how it's related to snapshotting non-file protocols (if this is even what you mean). What is your exact scenario, what does the existing code do in it, and how does this change improve it? An empty commit message is definitely not appropriate for such a change. In any case, using NULL as drv for bdrv_open() looks plain wrong. When passing drv bdrv_open tries to open the url as qcow2 or another plain file format and fail. What exactly is a plain file format and what is the difference between it and other file formats? Quorum is a protocol which encapsulate n BlockDriverState. So it's not a file format, neither plain nor otherwise. Anyway, this is not a problem description, let alone a description of your exact scenario. A scenario would be: A user want to create a new quorum snapshot made of n qcow2 snapshots. The user specify qcow2 as format so the new quorum files are created as qcow2. Then the code try to open the quorum file as a qcow2 file and it fail. Like this? base1 [file] --- base1 [qcow2] --\ base2 [file] --- base2 [qcow2] - quorum --- snap.qcow2 base3 [file] --- base3 [qcow2] --/ Or like this? (This is the only one where quorum is really used as a protocol) base1 [file] --\ base2 [file] - quorum --- base [qcow2] --- snap.qcow2 base3 [file] --/ Or even this one? base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ I think the last one is what you really want, but it's certainly not the case that is enabled by this patch. If qcow2 is specified as the format in the QMP command, why is it then wrong to try and open the file as qcow2? Why does it fail? What is the correct driver? It's correct to format the individual quorum files as qcow2 so specifying qcow2 in the qmp command seems ok. But opening a quorum: url which agregate these qcow2 files will fail if drv is the qcow2 driver. Would passing the quorum protocol driver to bdrv_open be ok ? A quorum: URL opened with the qcow2 driver should succeed and result in something like the second case mentioned above. Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ I think the last one is what you really want, but it's certainly not the case that is enabled by this patch. Yes I am trying to get this configuration: quorum always on top. Benoît If qcow2 is specified as the format in the QMP command, why is it then wrong to try and open the file as qcow2? Why does it fail? What is the correct driver? It's correct to format the individual quorum files as qcow2 so specifying qcow2 in the qmp command seems ok. But opening a quorum: url which agregate these qcow2 files will fail if drv is the qcow2 driver. Would passing the quorum protocol driver to bdrv_open be ok ? A quorum: URL opened with the qcow2 driver should succeed and result in something like the second case mentioned above. Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ I tried to default snapshot creation to qcow2 in quorum.c and specify quorum as format in the snapshot_blkdev command line. It doesn't work (crash). Do you have any idea on how to open while obtaining the third case ? Benoît
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Am 29.01.2013 16:03, schrieb Benoît Canet: base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ I tried to default snapshot creation to qcow2 in quorum.c and specify quorum as format in the snapshot_blkdev command line. It doesn't work (crash). Do you have any idea on how to open while obtaining the third case ? Wait for BlockBackends and then implement proper stay-on-top filters. Kevin
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
On 01/29/2013 07:27 AM, Benoît Canet wrote: base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ For that matter, I think it would be useful to have a quorum where the data is duplicated between different formats, such as: file1 [raw] --\ base2 [raw] --- snap2.qcow2 [qcow2] quorum base3 [qed] --- snap3.qed [qed] --/ In that case, you NEED to be able to pass in the protocol of each underlying device _as part of opening the quorum_, something like: quorum:2/3:file1[raw]:snap2.qcow2[qcow2]:snap3.qed[qed] or some such punctuation. And of course, that means you'd have to extend your escaping mechanism to allow escaping of whatever syntax you use for declaring a per-member format. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Le Tuesday 29 Jan 2013 à 16:21:43 (+0100), Kevin Wolf a écrit : Am 29.01.2013 16:03, schrieb Benoît Canet: base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\ base2 [file] --- base2 [qcow2] --- snap2.qcow2 - quorum base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/ I tried to default snapshot creation to qcow2 in quorum.c and specify quorum as format in the snapshot_blkdev command line. It doesn't work (crash). Do you have any idea on how to open while obtaining the third case ? Wait for BlockBackends and then implement proper stay-on-top filters. Any idea to disable properly snapshot creation for quorum and allowing it to be merged in a not so far future ? Benoît Kevin
[Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.
Signed-off-by: Benoit Canet ben...@irqsave.net
---
blockdev.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 0ce45c5..b1f388b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -800,7 +800,8 @@ void qmp_transaction(BlockdevActionList *dev_list, Error
**errp)
/* We will manually add the backing_hd field to the bs later */
states-new_bs = bdrv_new();
ret = bdrv_open(states-new_bs, new_image_file,
-flags | BDRV_O_NO_BACKING, drv);
+flags | BDRV_O_NO_BACKING,
+path_has_protocol(new_image_file) ? NULL : drv);
if (ret != 0) {
error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
goto delete_and_fail;
--
1.7.10.4
