Re: [Qemu-devel] [RFC PATCH 3/4] qmp: Add "snapshot=" option to nbd-server-add

[Fam Zheng <famz@redhat.com>]

On Wed, 08/21 15:02, Stefan Hajnoczi wrote:
> On Mon, Jul 29, 2013 at 12:25:31PM +0800, Fam Zheng wrote:
> > +/* create a point-in-time snapshot BDS from an existing BDS */
> > +static BlockDriverState *nbd_create_snapshot(BlockDriverState *orig_bs)
> > +{
> > +    int ret;
> > +    char filename[1024];
> > +    BlockDriver *drv;
> > +    BlockDriverState *bs;
> > +    QEMUOptionParameter *options;
> > +    Error *local_err = NULL;
> > +
> > +    bs = bdrv_new("");
> > +    ret = get_tmp_filename(filename, sizeof(filename));
> > +    if (ret < 0) {
> > +        goto err;
> 
> unlink(filename) is not safe if this function returns an error.
> 
> It is simpler if you get the temporary filename first and then do
> bdrv_new().
> 
> > +    }
> > +    drv = bdrv_find_format("qcow2");
> > +    if (drv < 0) {
> > +        goto err;
> > +    }
> > +    options = parse_option_parameters("", drv->create_options, NULL);
> > +    set_option_parameter_int(options, BLOCK_OPT_SIZE, bdrv_getlength(orig_bs));
> > +
> > +    ret = bdrv_create(drv, filename, options);
> 
> This is handy but only works if the QEMU process has permission to
> create temporary files.
> 
Yes, this is a shortcut, it has this limitation, but the good side is
it's easy to get and no other dependency.

To avoid creating file, we'll need blockdev-add to override backing_hd,
and blockdev-backup to use an existing BDS as target, then we simply use
current nbd-server-add to export it.

This series is still RFC, with above said, we still need to decide which
is the way we (QEMU and libvirt) want for 1.7.  any thoughts?

Thanks
Fam