[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 5/9] block: Allow child references on reopen

From: Alberto Garcia
Subject: Re: [Qemu-block] [PATCH 5/9] block: Allow child references on reopen
Date: Wed, 29 Aug 2018 16:56:20 +0200
User-agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu)

On Wed 29 Aug 2018 02:59:01 PM CEST, Max Reitz wrote:
>>> Specifically, that means when you don't specify @backing, the default
>>> chain will be opened and may replace the current one.
>> At the moment "backing" is the only child option that can be omitted,
>> all others must be specified. So if you leave "backing" out on reopen()
>> we have the following possibilities:
>>   a) We open the default backing file from disk. I don't think we want
>>      to open a new image on reopen(). Plus, what happens if the current
>>      backing image is the default one? Do we need to detect that? And
>>      what if it's the default image but has non-default options? The
>>      whole thing looks like a can of worms.
> True.
>>   b) We leave the current backing file untouched.
> Hm.  That doesn't make sense for blockdev-add, so you could argue that
> this behavior works as a default for reopen (because it cannot be
> "confused" with the blockdev-add default).
>>   c) We forbid it, and the user has to pass an explicit child, or NULL.
> That sounds good.
>> I chose (b) in this series. I suppose (c) could also be an option. But
>> (a) doesn't looke like a good idea.
> I agree with the last sentiment, but I'd prefer making it an error
> instead of choosing (b).  Yes, you could argue that choosing (b) as a
> default makes sense in a way, but in another, it just doesn't make
> sense.  (Because while (b) makes no sense for blockdev-add, (a) would
> make sense for both blockdev-add and reopen.  Sure, it's horrible to
> support, but from a user's POV, it makes sense.  So it's just
> confusing not to make that the default for reopen as well).
> But the most important point is that it's trivial for the user to keep
> the current backing file.  They just need to give the current
> backing's node-name.

Or NULL if there's no backing file, right?

The problem I see with this is that it would make the options quite
verbose. "backing" is an attribute of all BDSs, also protocol ones. I
suppose we can make "backing" optional in those, or is there any case
where it makes sense?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]