-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 24.08.2015 15:05, Alberto Garcia wrote: > If an image is opened with driver-specific options then attempting > to use snapshot_blkdev will fail with "Driver specified twice". > > The reason is that bs->filename is replaced with a full JSON object > by bdrv_refresh_filename() when such options are present: > > -drive if=virtio,file=hd0.qcow2,lazy-refcounts=on > > (qemu) info block virtio0: json:{"lazy-refcounts": "on", "driver": > "qcow2", "file": {"driver": "file", "filename": "hd0.qcow2"}} > (qcow2) > > A snapshot of that image will try to inherit its options, and that > includes parsing its filename when it is in the "json:" format. > > Since the JSON object includes a driver name, it will clash with > the one requested by the user in snapshot_blkdev, producing the > aforementioned error. > > Signed-off-by: Alberto Garcia <be...@igalia.com> --- block.c | 6 > ++++++ 1 file changed, 6 insertions(+)
User-specified options should always have precedence over any other option. The thing is, we consider the filename to be specified by the user. So it is actually correct that this option overrides the @drv parameter given to bdrv_open(), because that cannot be set by the user and is always set by qemu internally. The only way the user can set the driver (other than using a JSON filename) is by setting the "driver" option, and this will actually have precedence over the filename (see the comment at the end of this hunk), which is intended. So I think the problem here is not in bdrv_fill_options(), but rather in blockdev.c:external_snapshot_prepare(). This function should not pass the driver as the @drv parameter to bdrv_open(), but rather set the "driver" option in @options in order to mark this a user-specified option. Max > diff --git a/block.c b/block.c index d088ee0..b09de04 100644 --- > a/block.c +++ b/block.c @@ -1014,6 +1014,12 @@ static int > bdrv_fill_options(QDict **options, const char **pfilename, return > -EINVAL; } > > + /* We shouldn't use the driver from the filename if there > is + * one explicitly specified already */ + if > (drv) { + qdict_del(json_options, "driver"); + } > + /* Options given in the filename have lower priority than > options * specified directly */ qdict_join(*options, json_options, > false); > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJV22iAAAoJEDuxQgLoOKytPbUIAKLTo4wxxMSuYzASOONbnx7g PVrY7tcCGq2OEvAKirwufCJrGRQUvqqPci4k+zJzdtK2vAsxS6tYawY5bND00DBh wjQQIG7B16+ehEMVoyeLS7aslhaBbLMltcbQu7emS3vBMhQ1hUOkaP8IWxtQKRnE /2Afvi7f7pQ0gbCk0K13g2uGbHL0SE4GVCsGoAiFvIwNoFovHJ1chZlJQiMrAlMs 40HPaJapjjszDNc0mZ5arAoMEg9cZAd+THdJw11RAHdt3ty5/L048Qg3ui531i0O HPhT8qfhZA/pz6W88vd8wR6dbcSqyB6+uFAFjnV1/Cduj6KL4aZuPlHLdyU+Jg0= =zsDu -----END PGP SIGNATURE-----