On 29.03.2017 18:45, Markus Armbruster wrote: > Commit 831acdc "sheepdog: Implement bdrv_parse_filename()" and commit > d282f34 "sheepdog: Support blockdev-add" have different ideas on how > the QemuOpts parameters for the server address are named. Fix that. > While there, rename BlockdevOptionsSheepdog member addr to server, for > consistency with BlockdevOptionsSsh, BlockdevOptionsGluster, > BlockdevOptionsNbd. > > Commit 831acdc's example becomes > > --drive driver=sheepdog,server.host=fido,vdi=dolly > > instead of > > --drive driver=sheepdog,host=fido,vdi=dolly > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > block/sheepdog.c | 18 +++++++++--------- > qapi/block-core.json | 4 ++-- > 2 files changed, 11 insertions(+), 11 deletions(-)
OK, so let me get this straight: Before this patch, @addr was advertised as a runtime parameter that actually wasn't supported at all? (Because I can't see any place in block/sheepdog.c where it would be parsed.) This patch now renames @addr to @server and instead of actually parsing the option, it just removes the so-far convenience[1] options @host, @port, and @path and just fetches the from @server? If that is true, I can't say I like it the least. I guess it works for 2.9, but there should be a FIXME somewhere in this patch. Also, if you set any of server.* in bdrv_parse_filename(), you also have to set server.type. It's a SocketAddressFlat, .type is a required field. I know it's just for internal use, but that doesn't change the fact that it's an invalid SocketAddressFlat object without. Max [1] Originally there were apparently introduced for bdrv_parse_filename() -- but of course you can just use them in -drive directly...
signature.asc
Description: OpenPGP digital signature
