Am 31.08.2015 um 22:05 hat Eric Blake geschrieben: > On 08/31/2015 01:53 PM, Max Reitz wrote: > > > Design question: Would it make sense to instead add a "reference" mode > > to blockdev-snapshot-sync where you can specify a BDS's node-name > > instead of snapshot-file to use an existing BDS as the new top layer, > > ideally an empty one? > > Indeed - then blockdev-add can be used to create an unattached BDS (with > all appropriate options), and blockdev-snapshot-sync would then attach > that BDS as the snapshot-file that wraps an existing BDS (without > needing to worry about options).
Yes, this is what we should do. The existing blockdev-snapshot-sync should really have been called something like drive-snapshot, it doesn't belong in the blockdev-* family of commands which works only with existing nodes. > >> +++ b/qapi/block-core.json > >> @@ -697,11 +697,18 @@ > >> # > >> # @mode: #optional whether and how QEMU should create a new image, > >> default is > >> # 'absolute-paths'. > >> +# > >> +# @options: #optional options for the new device, with the following > >> +# restrictions for the fields: 'driver' must match the value > >> +# of @format, > > > > As said above, I'd rather make specifying both @options and @format > > exclusive. > > > > Maybe there is even some QAPI magic to enforce that (and for > > 'node-name', too), I don't know... > > Not that I know of at the moment, but not to say we can't add some. The > closest we can get is with a flat union, but that requires a > non-optional discriminator field. Maybe we can tweak qapi to make the > discriminator optional (with a default value). Thankfully, it sounds > like Markus' work on introspection would at least let management apps > learn about a new 'options' argument. Let's avoid such magic and instead add a new, clean blockdev-* style command. Maybe call it simply blockdev-snapshot; the -sync part was added because we knew it wouldn't be the final version of the command. Now we don't have any bdrv_open() in it any more that could by synchronous or asynchronous. Kevin
pgpcLBJEeyjiN.pgp
Description: PGP signature
