Re: [Qemu-devel] [PATCH 1/2] commit: Add top-node/base-node options

[Peter Krempa <pkrempa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

On Tue, Sep 04, 2018 at 16:42:17 +0200, Alberto Garcia wrote:
> On Tue 04 Sep 2018 04:17:30 PM CEST, Peter Krempa wrote:
> >> >> libvirt-3-format is read-write and all other node names are
> >> >> readonly in the above example.
> >> >> 
> >> >> The same also happens when using filenames:
> >> >> 
> >> >> {"execute":"block-commit",
> >> >>  "arguments" : {"device":"libvirt-3-format",
> >> >>                 "job-id":"libvirt-3-format",
> >> >>                 "top":"/var/lib/libvirt/images/rhel7.3.1483615252",
> >> >>                 "base":"/var/lib/libvirt/images/rhel7.3.1483605924",
> >> >>                 "auto-finalize":true,
> >> >>                 "auto-dismiss":false},
> >> >>  "id":"libvirt-13"}
> >> >> 
> >> >> {"id":"libvirt-13","error":{"class":"GenericError","desc":"Block node is read-only"}}
> >> >
> >> > I see what's happening here. So we have a graph like this:
> >> >
> >> >      guest device
> >> >           |
> >> >           v
> >> >     overlay-format -------> backing-format
> >> >     [read-only=off]         [read-only=on]
> >> >           |                        |
> >> >           v                        v
> >> >     overlay-proto           backing-proto
> >> >     [read-only=off]         [read-only=on]
> >> >
> >> > The difference between your -blockdev use and -drive is that you
> >> > explicitly specify the read-only option for backing-proto (and you use
> >> > a separate -blockdev option anyway), so it doesn't just inherit it
> >> > from backing-format.
> >> 
> >> Are these format and protocol block devices opened with four separate
> >> -blockdev parameters? Is that how libvirt does it?
> >
> > Yes. This goes along with the fact that for 'blockdev-create' you need
> > to blockdev-add the file which you want to format, but the formatted
> > file is not automatically added.
> >
> > If we'd use the approach where the protocol layer is opened as part of
> > the format layer it would complicate the snapshot code where we need
> > to add a file and then format it to qcow2. It would mean that we'd
> > have to blockdev-add a file, format it via blockdev-create, then
> > blockdev-del it and open it together with the format layer. Otherwise
> > the disk hot-unplug code would be plain crazy.
> 
> Do you need to add the protocol layer in order to format it, though? :-?
> 
> (I'm just trying to understand how this works, I'm not too familiar with
> blockdev-create)
> 
> {'execute': 'blockdev-create',
>  'arguments': {'job-id': 'job0',
>                'options': {'driver': 'file',
>                            'filename': 'test.qcow2',
>                            'size': 0}}}
> {'execute': 'job-dismiss', 'arguments': {'id': 'job0'}}
> 
> {'execute': 'blockdev-create',
>  'arguments': {'job-id': 'job1',
>                'options': { 'driver': 'qcow2',
>                             'size': 1048576,
>                             'file': {'driver': 'file',
>                                      'filename': 'test.qcow2'}}}}
> {'execute': 'job-dismiss', 'arguments': {'id': 'job1'}}

Honestly I've reused the existing approach and did not try without
actually adding the protocol layer.

I remember being told some time ago to specify both layers explicitly.
Since it's not yet enabled in libvirt we theoretically could change to
one -blockdev for format+protocol but in that case we need some kind
of guarantee that every (even new) feature will work with it.

Switching between those approaches once we enable it upstream will not
be possible without adding a lot of compatibility code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]