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

Re: [libvirt] [PATCH 7/8] Given virDomainDef parser & formatter their own flags



On Tue, Nov 18, 2014 at 05:59:54PM +0000, Daniel P. Berrange wrote:
The virDomainDefParse* and virDomainDefFormat* methods both
accept the VIR_DOMAIN_XML_* flags defined in the public API,
along with a set of other VIR_DOMAIN_XML_INTERNAL_* flags
defined in domain_conf.c.

This is seriously confusing & error prone for a number of
reasons:

- VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_MIGRATABLE and
  VIR_DOMAIN_XML_UPDATE_CPU are only relevant for the
  formatting operation
- VIR_DOMAIN_XML_UPDATE_CPU is not in fact handled by
  the domain_conf.c code at all - it must be dealt with
  by the driver code
- Some of the VIR_DOMAIN_XML_INTERNAL_* flags only apply
  to parse or to format, but not both.

This patch cleanly separates out the flags. There are two
distint VIR_DOMAIN_DEF_PARSE_* and VIR_DOMAIN_DEF_FORMAT_*
flags that are used by the corresponding methods. The
VIR_DOMAIN_XML_* flags received via public API calls must
be converted to the VIR_DOMAIN_DEF_FORMAT_* flags where
needed.

The various calls to virDomainDefParse which hardcoded the
use of the VIR_DOMAIN_XML_INACTIVE flag change to use the
VIR_DOMAIN_DEF_PARSE_INACTIVE flag.
---
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5f4b9f6..2dace76 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18690,8 +18661,7 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def)
    return false;
}

-/* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
- * whereas the public version cannot.  Also, it appends to an existing
+/* This internal version  appends to an existing
Biggest nit-pick ever     ^^ two spaces here.

/me hides under the rock

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 530a3ca..98497d6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2389,42 +2389,74 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
                                  virDomainObjPtr dom);

+typedef enum {
+    /* parse internal domain status information */
+    VIR_DOMAIN_DEF_PARSE_STATUS          = 1 << 0,
+    VIR_DOMAIN_DEF_PARSE_INACTIVE        = 1 << 2,
+    /* parse <actual> element */
+    VIR_DOMAIN_DEF_PARSE_ACTUAL_NET      = 1 << 3,
+    /* parse original states of host PCI device */
+    VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES = 1 << 4,
+    VIR_DOMAIN_DEF_PARSE_ALLOW_ROM       = 1 << 5,
+    VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT      = 1 << 6,
+    VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST    = 1 << 7,
+    /* parse only source half of <disk> */
+    VIR_DOMAIN_DEF_PARSE_DISK_SOURCE     = 1 << 8,
+} virDomainDefParseFlags;
+
+typedef enum {
+    VIR_DOMAIN_DEF_FORMAT_SECURE          = 1 << 0,
+    VIR_DOMAIN_DEF_FORMAT_INACTIVE        = 1 << 1,
+    VIR_DOMAIN_DEF_FORMAT_MIGRATABLE      = 1 << 2,
+    /* format internal domain status information */
+    VIR_DOMAIN_DEF_FORMAT_STATUS          = 1 << 3,
+    /* format <actual> element */
+    VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET      = 1 << 4,
+    /* format original states of host PCI device */
+    VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES = 1 << 5,
+    VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM       = 1 << 6,
+    VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT      = 1 << 7,
+    VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST    = 1 << 8,
+} virDomainDefFormatFlags;
+
virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
                                              const virDomainDef *def,
                                              virCapsPtr caps,
                                              virDomainXMLOptionPtr xmlopt,
                                              unsigned int flags);
virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr,
-                                                const virDomainDef *def,
-                                                virDomainXMLOptionPtr xmlopt,
-                                                unsigned int flags);
+                                                 const virDomainDef *def,
+                                                 virDomainXMLOptionPtr xmlopt,
+                                                 unsigned int flags);
virDomainDefPtr virDomainDefParseString(const char *xmlStr,
-                                        virCapsPtr caps,
-                                        virDomainXMLOptionPtr xmlopt,
-                                        unsigned int expectedVirtTypes,
-                                        unsigned int flags);
+                                         virCapsPtr caps,
+                                         virDomainXMLOptionPtr xmlopt,
+                                         unsigned int expectedVirtTypes,
+                                         unsigned int flags);
virDomainDefPtr virDomainDefParseFile(const char *filename,
-                                      virCapsPtr caps,
-                                      virDomainXMLOptionPtr xmlopt,
-                                      unsigned int expectedVirtTypes,
-                                      unsigned int flags);
+                                       virCapsPtr caps,
+                                       virDomainXMLOptionPtr xmlopt,
+                                       unsigned int expectedVirtTypes,
+                                       unsigned int flags);
virDomainDefPtr virDomainDefParseNode(xmlDocPtr doc,
-                                      xmlNodePtr root,
-                                      virCapsPtr caps,
-                                      virDomainXMLOptionPtr xmlopt,
-                                      unsigned int expectedVirtTypes,
-                                      unsigned int flags);
+                                       xmlNodePtr root,
+                                       virCapsPtr caps,
+                                       virDomainXMLOptionPtr xmlopt,
+                                       unsigned int expectedVirtTypes,
+                                       unsigned int flags);


It looks like this part of this hunk just screws up indentation.  If
this is not a mistake, then somewhere along the way the mail itself
got screwed.

Martin

Attachment: signature.asc
Description: Digital signature


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