On Tue 01 Sep 2015 01:31:11 PM CEST, Kevin Wolf <[email protected]> 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. Sounds like a good idea, thanks for the feedback ! >> >> +# @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. This is not necessary if we go for the "reference" mode proposed by Markus, since the "options" parameter would disappear. > 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. Would you then prefer me to create a new command instead of extending the existing one? What would be the benefit (other than a better name)? Berto
