Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

[Luiz Capitulino]

On Thu, 28 Apr 2011 15:21:41 +0200
Jes Sorensen <address@hidden> wrote:

> On 04/27/11 17:05, Luiz Capitulino wrote:
> >> +If a new image file is specified, the new image file will become the
> >> > +new root image. If format is specified, the snapshot file will be
> >> > +created in that format. Otherwise the snapshot will be internal!
> >> > +(currently unsupported).
> > Sorry for the stupid question, but what's a "new root image"? Also, all
> > these assumptions seem human features to me, as it can save some typing
> > and I can poke around to see where the snapshots are stored.
> > 
> > All arguments should be mandatory in QMP, IMO.
> 
> Sorry, but there is absolutely no reason to make all arguments
> mandatory. Sure it can be done, but the only result is a separate
> handling function for it, so we got more almost identical, but still
> different code to maintain.

We shouldn't compromise our external interface quality because of
implementation details. What I'm really asking here is whether this is
a good command for our management tools.

For example, I've just realized that the new root image is going to be
automatically created after the first call to this command, and subsequent
calls w/o the snapshot file name will re-use that file. Is that correct?

Also note the optional format usage, the command (randomly) picks qcow2 if
the format is not given. What happens if I pass a raw image and don't specify
the format? Will it work as it works for qcow2?

I'm not exactly asking for mandatory arguments. For the format argument for
example, we could try to auto-detect the format (is it possible)? And then
we could fail with a meaningful error message.

And, I also would like to hear from Anthony, as he's picking up QMP maintenance.

> > Finally, what's the expect behavior when -snapshot is used? I'm getting
> > this:
> > 
> >  (qemu) snapshot_blkdev ide0-hd0 snap-test
> >  Could not open '/tmp/vl.6w8YXA'
> >  (qemu)
> 
> What type of file system is your /tmp?

ext4

> You need to provide full path to
> the snapshot file if you don't want it created next to where your qemu
> binary is being executed.

I'm not running in /tmp.

> > At first, I don't see why we shouldn't generate the live snapshot, but 
> > anyway,
> > any special behavior like this should be noted in the section called Notes
> > in the command's documentation.
> > 
> 
> I don't follow this at all, please elaborate.

Any kind of limitation should be noted in the documentation.