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

Re: [libvirt] [PATCHv2 3/3] snapshot: rename an enum



On Thu, Aug 23, 2012 at 02:54:16PM -0600, Eric Blake wrote:
> The name 'virDomainDiskSnapshot' didn't fit in with our normal
> conventions of using a prefix hinting that it is related to a
> virDomainSnapshotPtr.  Also, a future patch will reuse the
> enum for declaring where the VM memory is stored.
> 
> * src/conf/snapshot_conf.h (virDomainDiskSnapshot): Rename...
> (virDomainSnapshotLocation): ...to this.
> (_virDomainSnapshotDiskDef): Update clients.
> * src/conf/domain_conf.h (_virDomainDiskDef): Likewise.
> * src/libvirt_private.syms (domain_conf.h): Likewise.
> * src/conf/domain_conf.c (virDomainDiskDefParseXML)
> (virDomainDiskDefFormat): Likewise.
> * src/conf/snapshot_conf.c: (virDomainSnapshotDiskDefParseXML)
> (virDomainSnapshotAlignDisks, virDomainSnapshotDefFormat):
> Likewise.
> * src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare)
> (qemuDomainSnapshotCreateSingleDiskActive)
> (qemuDomainSnapshotCreateDiskActive, qemuDomainSnapshotCreateXML):
> Likewise.
> ---
>  src/conf/domain_conf.c   |  8 ++++----
>  src/conf/domain_conf.h   |  2 +-
>  src/conf/snapshot_conf.c | 16 +++++++++-------
>  src/conf/snapshot_conf.h | 16 ++++++++--------
>  src/libvirt_private.syms |  4 ++--
>  src/qemu/qemu_driver.c   | 19 ++++++++++---------
>  6 files changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e3d04a7..bcaa9b1 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3766,7 +3766,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>      }
> 
>      if (snapshot) {
> -        def->snapshot = virDomainDiskSnapshotTypeFromString(snapshot);
> +        def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot);
>          if (def->snapshot <= 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("unknown disk snapshot setting '%s'"),
> @@ -3774,7 +3774,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>              goto error;
>          }
>      } else if (def->readonly) {
> -        def->snapshot = VIR_DOMAIN_DISK_SNAPSHOT_NO;
> +        def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
>      }
> 
>      if (rawio) {
> @@ -11362,9 +11362,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
>          }
>      }
>      if (def->snapshot &&
> -        !(def->snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO && def->readonly))
> +        !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && def->readonly))
>          virBufferAsprintf(buf, " snapshot='%s'",
> -                          virDomainDiskSnapshotTypeToString(def->snapshot));
> +                          virDomainSnapshotLocationTypeToString(def->snapshot));
>      virBufferAddLit(buf, ">\n");
> 
>      if (def->driverName || def->driverType || def->cachemode ||
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ef4feaa..9ee57e1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -578,7 +578,7 @@ struct _virDomainDiskDef {
>      int ioeventfd;
>      int event_idx;
>      int copy_on_read;
> -    int snapshot; /* enum virDomainDiskSnapshot, snapshot_conf.h */
> +    int snapshot; /* enum virDomainSnapshotLocation, snapshot_conf.h */
>      int startupPolicy; /* enum virDomainStartupPolicy */
>      unsigned int readonly : 1;
>      unsigned int shared : 1;
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 894a74c..b9eb74c 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -51,7 +51,7 @@
> 
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN_SNAPSHOT
> 
> -VIR_ENUM_IMPL(virDomainDiskSnapshot, VIR_DOMAIN_DISK_SNAPSHOT_LAST,
> +VIR_ENUM_IMPL(virDomainSnapshotLocation, VIR_DOMAIN_SNAPSHOT_LOCATION_LAST,
>                "default",
>                "no",
>                "internal",
> @@ -120,7 +120,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
> 
>      snapshot = virXMLPropString(node, "snapshot");
>      if (snapshot) {
> -        def->snapshot = virDomainDiskSnapshotTypeFromString(snapshot);
> +        def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot);
>          if (def->snapshot <= 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("unknown disk snapshot setting '%s'"),
> @@ -144,7 +144,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
>      }
> 
>      if (!def->snapshot && (def->file || def->driverType))
> -        def->snapshot = VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL;
> +        def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
> 
>      ret = 0;
>  cleanup:
> @@ -390,14 +390,16 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
>              disk->snapshot = disk_snapshot;
>          } else if (disk_snapshot && require_match &&
>                     disk->snapshot != disk_snapshot) {
> -            const char *tmp = virDomainDiskSnapshotTypeToString(disk_snapshot);
> +            const char *tmp;
> +
> +            tmp = virDomainSnapshotLocationTypeToString(disk_snapshot);
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("disk '%s' must use snapshot mode '%s'"),
>                             disk->name, tmp);
>              goto cleanup;
>          }
>          if (disk->file &&
> -            disk->snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) {
> +            disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("file '%s' for disk '%s' requires "
>                               "use of external snapshot mode"),
> @@ -445,7 +447,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
>      for (i = 0; i < def->ndisks; i++) {
>          virDomainSnapshotDiskDefPtr disk = &def->disks[i];
> 
> -        if (disk->snapshot == VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL &&
> +        if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
>              !disk->file) {
>              const char *original = def->dom->disks[i]->src;
>              const char *tmp;
> @@ -534,7 +536,7 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid,
>              virBufferEscapeString(&buf, "    <disk name='%s'", disk->name);
>              if (disk->snapshot)
>                  virBufferAsprintf(&buf, " snapshot='%s'",
> -                                  virDomainDiskSnapshotTypeToString(disk->snapshot));
> +                                  virDomainSnapshotLocationTypeToString(disk->snapshot));
>              if (disk->file || disk->driverType) {
>                  virBufferAddLit(&buf, ">\n");
>                  if (disk->driverType)
> diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
> index 314c4d1..7d46a82 100644
> --- a/src/conf/snapshot_conf.h
> +++ b/src/conf/snapshot_conf.h
> @@ -29,13 +29,13 @@
> 
>  /* Items related to snapshot state */
> 
> -enum virDomainDiskSnapshot {
> -    VIR_DOMAIN_DISK_SNAPSHOT_DEFAULT = 0,
> -    VIR_DOMAIN_DISK_SNAPSHOT_NO,
> -    VIR_DOMAIN_DISK_SNAPSHOT_INTERNAL,
> -    VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
> +enum virDomainSnapshotLocation {
> +    VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT = 0,
> +    VIR_DOMAIN_SNAPSHOT_LOCATION_NONE,
> +    VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL,
> +    VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
> 
> -    VIR_DOMAIN_DISK_SNAPSHOT_LAST
> +    VIR_DOMAIN_SNAPSHOT_LOCATION_LAST
>  };
> 
>  enum virDomainSnapshotState {
> @@ -50,7 +50,7 @@ typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr;
>  struct _virDomainSnapshotDiskDef {
>      char *name; /* name matching the <target dev='...' of the domain */
>      int index; /* index within snapshot->dom->disks that matches name */
> -    int snapshot; /* enum virDomainDiskSnapshot */
> +    int snapshot; /* enum virDomainSnapshotLocation */
>      char *file; /* new source file when snapshot is external */
>      char *driverType; /* file format type of new file */
>  };
> @@ -151,7 +151,7 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots,
>                             virDomainSnapshotPtr **snaps,
>                             unsigned int flags);
> 
> -VIR_ENUM_DECL(virDomainDiskSnapshot)
> +VIR_ENUM_DECL(virDomainSnapshotLocation)
>  VIR_ENUM_DECL(virDomainSnapshotState)
> 
>  #endif /* __SNAPSHOT_CONF_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c339664..27eb43e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -341,8 +341,6 @@ virDomainDiskIoTypeToString;
>  virDomainDiskPathByName;
>  virDomainDiskRemove;
>  virDomainDiskRemoveByName;
> -virDomainDiskSnapshotTypeFromString;
> -virDomainDiskSnapshotTypeToString;
>  virDomainDiskTypeFromString;
>  virDomainDiskTypeToString;
>  virDomainEmulatorPinAdd;
> @@ -485,6 +483,8 @@ virDomainSnapshotFindByName;
>  virDomainSnapshotForEach;
>  virDomainSnapshotForEachChild;
>  virDomainSnapshotForEachDescendant;
> +virDomainSnapshotLocationTypeFromString;
> +virDomainSnapshotLocationTypeToString;
>  virDomainSnapshotObjListGetNames;
>  virDomainSnapshotObjListNum;
>  virDomainSnapshotObjListRemove;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c1cfa5a..64c407d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10558,7 +10558,7 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>          virDomainSnapshotDiskDefPtr disk = &def->disks[i];
> 
>          switch (disk->snapshot) {
> -        case VIR_DOMAIN_DISK_SNAPSHOT_INTERNAL:
> +        case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
>              if (active) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                 _("active qemu domains require external disk "
> @@ -10578,7 +10578,7 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>              found = true;
>              break;
> 
> -        case VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL:
> +        case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
>              if (!disk->driverType) {
>                  if (!(disk->driverType = strdup("qcow2"))) {
>                      virReportOOMError();
> @@ -10610,10 +10610,10 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>              external++;
>              break;
> 
> -        case VIR_DOMAIN_DISK_SNAPSHOT_NO:
> +        case VIR_DOMAIN_SNAPSHOT_LOCATION_NONE:
>              break;
> 
> -        case VIR_DOMAIN_DISK_SNAPSHOT_DEFAULT:
> +        case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT:
>          default:
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("unexpected code path"));
> @@ -10668,7 +10668,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
>      char *origdriver = NULL;
>      bool need_unlink = false;
> 
> -    if (snap->snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) {
> +    if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("unexpected code path"));
>          return -1;
> @@ -10919,7 +10919,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>      for (i = 0; i < snap->def->ndisks; i++) {
>          virDomainDiskDefPtr persistDisk = NULL;
> 
> -        if (snap->def->disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO)
> +        if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
>              continue;
>          if (vm->newDef) {
>              int indx = virDomainDiskIndexByName(vm->newDef,
> @@ -10949,7 +10949,8 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>              while (--i >= 0) {
>                  virDomainDiskDefPtr persistDisk = NULL;
> 
> -                if (snap->def->disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO)
> +                if (snap->def->disks[i].snapshot ==
> +                    VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
>                      continue;
>                  if (vm->newDef) {
>                      int indx = virDomainDiskIndexByName(vm->newDef,
> @@ -11178,7 +11179,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>          }
>          if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && def->dom) {
>              if (virDomainSnapshotAlignDisks(def,
> -                                            VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
> +                                            VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
>                                              false) < 0)
>                  goto cleanup;
>          }
> @@ -11199,7 +11200,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>                  goto cleanup;
>              }
>              if (virDomainSnapshotAlignDisks(def,
> -                                            VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
> +                                            VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
>                                              false) < 0)
>                  goto cleanup;
>              if (qemuDomainSnapshotDiskPrepare(vm, def, &flags) < 0)

  ACK, the only annoyance is that the name being longuer the simple
substitution leads to lines over 80 characters, I would be a proponent
of dropping the indentation to the opening ( for the sake of keeping
everything on one 80 char line

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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