Am 19.02.2019 um 00:57 hat Max Reitz geschrieben: > On 31.01.19 18:55, Kevin Wolf wrote: > > This adds a .bdrv_open option to specify the external data file node. > > > > Signed-off-by: Kevin Wolf <[email protected]> > > --- > > qapi/block-core.json | 3 ++- > > block/qcow2.h | 4 +++- > > block/qcow2.c | 25 +++++++++++++++++++++++-- > > 3 files changed, 28 insertions(+), 4 deletions(-) > > [...] > > > diff --git a/block/qcow2.h b/block/qcow2.h > > index c161970882..e2114900b4 100644 > > --- a/block/qcow2.h > > +++ b/block/qcow2.h > > [...] > > > @@ -205,7 +206,8 @@ enum { > > QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, > > > > QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY > > - | QCOW2_INCOMPAT_CORRUPT, > > + | QCOW2_INCOMPAT_CORRUPT > > + | QCOW2_INCOMPAT_DATA_FILE, > > This hunk seems to belong somewhere else.
Isn't this the first patch that actually allows opening images that have
QCOW2_INCOMPAT_DATA_FILE set in their header?
> > };
> >
> > /* Compatible feature bits */
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index ac9934b3ed..376232d3f0 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1441,8 +1441,22 @@ static int coroutine_fn
> > qcow2_do_open(BlockDriverState *bs, QDict *options,
> > goto fail;
> > }
> >
> > - /* TODO Open external data file */
> > - s->data_file = bs->file;
> > + /* Open external data file */
> > + if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
> > + s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
> > + &child_file, false, errp);
> > + if (!s->data_file) {
> > + ret = -EINVAL;
> > + goto fail;
> > + }
> > + } else if (qdict_get(options, QCOW2_OPT_DATA_FILE)) {
>
> I get the idea, but this isn't crumpled so this key may not exist (but
> data-file.driver and data-file.filename may). Of course the fact that
> these options remain unused will be caught by the block layer, but that
> makes the error message below a bit less useful.
Hmm, good point... So you'd just leave out the check and always let the
block layer complain (with a less than helpful message)? Or are you
suggesting I should try and catch all cases somehow, even if that makes
things quite a bit more complicated?
Kevin
signature.asc
Description: PGP signature
