Re: RFC: Exposing backing chains in <domain> XML

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

 



On 03/12/2014 05:34 PM, Eric Blake wrote:
> 
> Code-wise, I'm looking at splitting 'struct _virDomainDiskDef' into two
> parts.  The outermost part is _virDomainDiskDef, which tracks anything
> tied to the guest view, or to the device as a whole (<target>, <alias>,
> <address>); the inner part is a new _virDomainDiskSrcDef, which tracks
> anything related to a host view (node name, <source>, <driver>,
> <backingStore>), and where each backingStore is also a
> _virDomainDiskSrcDef, as a recursive structure - we just special case
> the output so that the first _virDomainDiskSrcDef feeds the XML of
> <disk> element, while all other _virDomainDiskSrcDef feed the XML of a
> <backingStore>.

This won't compile, but here's the split I'm currently envisioning.
Also, Peter reminded me on IRC that it is going to be nicer if the
host-side source resource can be reusable in the src/util/virstorage
framework, which means I need to move the inner struct out of
conf/domain_conf.h (conf/ can use util/, but util/ cannot use conf/):

diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index 37191a8..5a3cd77 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -688,108 +688,119 @@ enum virDomainDiskSourcePoolMode {
...
 };
 typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;

-/* Stores the virtual disk configuration */
-struct _virDomainDiskDef {
-    int type;
-    int device;
-    int bus;
+struct _virDomainDiskSourceDef {
+    int type; /* enum virDomainDiskType */
     char *src;
-    char *dst;
-    int tray_status;
-    int removable;
-    int protocol;
+    int protocol; /* enum virDomainDiskProtocol */
     size_t nhosts;
     virDomainDiskHostDefPtr hosts;
     virDomainDiskSourcePoolDefPtr srcpool;
     struct {
         char *username;
         int secretType; /* enum virDomainDiskSecretType */
         union {
             unsigned char uuid[VIR_UUID_BUFLEN];
             char *usage;
         } secret;
     } auth;
+    virStorageEncryptionPtr encryption;
     char *driverName;
     int format; /* enum virStorageFileFormat */
     virStorageFileMetadataPtr backingChain;

+    size_t nseclabels;
+    virSecurityDeviceLabelDefPtr *seclabels;
+
+    bool noBacking;
+    virDomainDiskSourceDefPtr backing;
+};
+typedef struct _virDomainDiskSourceDef virDomainDiskSourceDef;
+typedef virDomainDiskSourceDef *virDomainDiskSourceDefPtr;
+
+/* Stores the virtual disk configuration */
+struct _virDomainDiskDef {
+    virDomainDiskSourceDef src;
+
+    int device; /* enum virDomainDiskDevice */
+    int bus; /* enum virDomainDiskBus */
+    char *dst;
+    int tray_status;
+    int removable;
+    virStorageFileMetadataPtr backingChain;
+
     char *mirror;
     int mirrorFormat; /* enum virStorageFileFormat */
     bool mirroring;

     struct {
         unsigned int cylinders;
         unsigned int heads;
         unsigned int sectors;
         int trans;
     } geometry;

     struct {
         unsigned int logical_block_size;
         unsigned int physical_block_size;
     } blockio;

     virDomainBlockIoTuneInfo blkdeviotune;

     char *serial;
     char *wwn;
     char *vendor;
     char *product;
     int cachemode;
     int error_policy;  /* enum virDomainDiskErrorPolicy */
     int rerror_policy; /* enum virDomainDiskErrorPolicy */
     int iomode;
     int ioeventfd;
     int event_idx;
     int copy_on_read;
     int snapshot; /* enum virDomainSnapshotLocation, snapshot_conf.h */
     int startupPolicy; /* enum virDomainStartupPolicy */
     bool readonly;
     bool shared;
     bool transient;
     virDomainDeviceInfo info;
-    virStorageEncryptionPtr encryption;
     bool rawio_specified;
     int rawio; /* no = 0, yes = 1 */
     int sgio; /* enum virDomainDeviceSGIO */
     int discard; /* enum virDomainDiskDiscard */
-
-    size_t nseclabels;
-    virSecurityDeviceLabelDefPtr *seclabels;
 };



If I did the split right, then everything that is per-device remains in
the outer struct, and everything that is per-file is in the inner
struct.  noBacking is required to know whether 'backing==NULL' implies
<backingStore/> as an explicit end of chain, vs. omitting the subelement
from older versions or user input that is still expecting libvirt to
populate the backing chain into the xml.  Another interesting
observation - we have obviously not done much with chains of encrypted
volumes, because just a little thought makes it obvious that <auth> and
storageEncryption must be per-file attributes (it is feasible to have a
chain of two separate encrypted qcow2 images[*], where the two images
need SEPARATE passwords) while the current design of only one <auth> per
device doesn't cope.  Similarly, we can finally express the fact that
the security label on backing stores is readonly while the top-most file
is read-write; as well as designate when we have changed a backing store
to read-write in order to update metadata such as during a commit
operation (there are some FIXMEs in qemu_driver about knowing when to
revert read-write privileges of backing stores if a block commit extends
over a libvirtd restart).

[*] Of course, I must give the caveat that I'd highly recommend AGAINST
using qcow2 encryption - it is known to be a lousy implementation, when
compared to LUKS.

In making the proposed split, I noticed that we've abused the <driver>
element to contain a hodgepodge of things that are per-device (for
example, cache is a per-device setting, while format is a per-file
setting), so I'm now trying to figure out how to tweak the XML to
express the difference.  I may end up keeping <driver> only at the top
level, and adding a new <format> subelement inside <backingStore>, then
for back-compat reasons duplicate <driver format='...'/> and <format> at
the top level, or teaching the disk source formatter to merely append in
a string of device-level attributes when formatting the active disk of
the chain.

Peter, how does this split coincide with what you were looking at?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]