Re: [libvirt] [RFC PATCH 02/30] qemu: domain: Format storage source node names into private data

Am 20.04.2018 um 09:45 hat Peter Krempa geschrieben: > On Fri, Apr 20, 2018 at 09:30:16 +0200, Kevin Wolf wrote: > > Am 19.04.2018 um 17:24 hat Peter Krempa geschrieben: > > > Save and restore node names if we know them in the status XML so that we > > > don't need to recalculate them or don't lose them in some cases. > > > > > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > > > --- > > > src/qemu/qemu_domain.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > > > index e2a8450e2e..224b3b0478 100644 > > > --- a/src/qemu/qemu_domain.c > > > +++ b/src/qemu/qemu_domain.c > > > @@ -1934,6 +1934,9 @@ static int > > > qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, > > > virStorageSourcePtr src) > > > { > > > + src->nodestorage = virXPathString("string(./nodename/@storage)", ctxt); > > > + src->nodeformat = virXPathString("string(./nodename/@format)", ctxt); > > > + > > > if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) > > > return -1; > > > > This format doesn't look very future-proof. I can see that using > > x->nodestorage and x->nodeformat internally are probably easier for now, > > but while the assumption of exactly two layers (except for backing > > files) is probably correct for most cases, it will become less and less > > common the more we use filter drivers. > > > > Representing an arbitrary tree (or even non-tree graph) in the XML may > > be unpractical now, but please be aware that you'll probably need this > > change in the long run. > > Yes. It will need to change e.g. if we allow IO throttling for other > than the top layer. On the other hand it would be impractical to change > to the topology qemu uses where a storage file is two layers minimum, so > we will need to track everyting for one layer of XML. The two layers minimum is not correct as far as QEMU is concerned. A two-layer setup is what -drive creates by default if you don't specify a protocol driver as driver=... (or format=... for older versions) explicitly, but if you want, you can directly assign a 'file' node to a guest device without adding a format layer on top. > As the above uses attributes in the XML we can add an arbitrary amount > of them if required. Quorum takes an array of children, and VMDK needs a number of extents that depends on the image file, so attributes probably won't do. If

we didn't have to consider compatibility, I think I'd choose something like:

<child-nodes> <child-node name="file" ref="my-file-node" /> <child-node name="backing" ref="my-backing-node" /> </child-nodes> Kevin