qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Remove code duplication from savevm instance_id creation code.
@ 2010-11-07 12:16 Gleb Natapov
  2010-11-10 16:01 ` [Qemu-devel] " Alex Williamson
  0 siblings, 1 reply; 2+ messages in thread
From: Gleb Natapov @ 2010-11-07 12:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.williamson, quintela

Code in register_savevm_live() and vmstate_register_with_alias_id()
looks identical except idstr used. Move into separate function.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/savevm.c b/savevm.c
index cf56121..6ca6920 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1080,6 +1080,33 @@ static int calculate_compat_instance_id(const char *idstr)
     return instance_id;
 }
 
+static void setup_instance_id(DeviceState *dev, const char *idstr,
+                              int instance_id, SaveStateEntry *se)
+{
+    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
+        char *id = dev->parent_bus->info->get_dev_path(dev);
+        if (id) {
+            pstrcpy(se->idstr, sizeof(se->idstr), id);
+            pstrcat(se->idstr, sizeof(se->idstr), "/");
+            qemu_free(id);
+
+            se->compat = qemu_mallocz(sizeof(CompatEntry));
+            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), idstr);
+            se->compat->instance_id = instance_id == -1 ?
+                         calculate_compat_instance_id(idstr) : instance_id;
+            instance_id = -1;
+        }
+    }
+    pstrcat(se->idstr, sizeof(se->idstr), idstr);
+
+    if (instance_id == -1) {
+        se->instance_id = calculate_new_instance_id(se->idstr);
+    } else {
+        se->instance_id = instance_id;
+    }
+    assert(!se->compat || se->instance_id == 0);
+}
+
 /* TODO: Individual devices generally have very little idea about the rest
    of the system, so instance_id should be removed/replaced.
    Meanwhile pass -1 as instance_id if you do not already have a clearly
@@ -1107,28 +1134,8 @@ int register_savevm_live(DeviceState *dev,
     se->vmsd = NULL;
     se->no_migrate = 0;
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
-        if (id) {
-            pstrcpy(se->idstr, sizeof(se->idstr), id);
-            pstrcat(se->idstr, sizeof(se->idstr), "/");
-            qemu_free(id);
-
-            se->compat = qemu_mallocz(sizeof(CompatEntry));
-            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), idstr);
-            se->compat->instance_id = instance_id == -1 ?
-                         calculate_compat_instance_id(idstr) : instance_id;
-            instance_id = -1;
-        }
-    }
-    pstrcat(se->idstr, sizeof(se->idstr), idstr);
+    setup_instance_id(dev, idstr, instance_id, se);
 
-    if (instance_id == -1) {
-        se->instance_id = calculate_new_instance_id(se->idstr);
-    } else {
-        se->instance_id = instance_id;
-    }
-    assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
     QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
     return 0;
@@ -1217,28 +1224,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->vmsd = vmsd;
     se->alias_id = alias_id;
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
-        if (id) {
-            pstrcpy(se->idstr, sizeof(se->idstr), id);
-            pstrcat(se->idstr, sizeof(se->idstr), "/");
-            qemu_free(id);
-
-            se->compat = qemu_mallocz(sizeof(CompatEntry));
-            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), vmsd->name);
-            se->compat->instance_id = instance_id == -1 ?
-                         calculate_compat_instance_id(vmsd->name) : instance_id;
-            instance_id = -1;
-        }
-    }
-    pstrcat(se->idstr, sizeof(se->idstr), vmsd->name);
+    setup_instance_id(dev, vmsd->name, instance_id, se);
 
-    if (instance_id == -1) {
-        se->instance_id = calculate_new_instance_id(se->idstr);
-    } else {
-        se->instance_id = instance_id;
-    }
-    assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
     QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
     return 0;
--
			Gleb.

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Qemu-devel] Re: [PATCH] Remove code duplication from savevm instance_id creation code.
  2010-11-07 12:16 [Qemu-devel] [PATCH] Remove code duplication from savevm instance_id creation code Gleb Natapov
@ 2010-11-10 16:01 ` Alex Williamson
  0 siblings, 0 replies; 2+ messages in thread
From: Alex Williamson @ 2010-11-10 16:01 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: qemu-devel, quintela

On Sun, 2010-11-07 at 14:16 +0200, Gleb Natapov wrote:
> Code in register_savevm_live() and vmstate_register_with_alias_id()
> looks identical except idstr used. Move into separate function.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>

looks ok to me.

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> diff --git a/savevm.c b/savevm.c
> index cf56121..6ca6920 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1080,6 +1080,33 @@ static int calculate_compat_instance_id(const char *idstr)
>      return instance_id;
>  }
>  
> +static void setup_instance_id(DeviceState *dev, const char *idstr,
> +                              int instance_id, SaveStateEntry *se)
> +{
> +    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
> +        char *id = dev->parent_bus->info->get_dev_path(dev);
> +        if (id) {
> +            pstrcpy(se->idstr, sizeof(se->idstr), id);
> +            pstrcat(se->idstr, sizeof(se->idstr), "/");
> +            qemu_free(id);
> +
> +            se->compat = qemu_mallocz(sizeof(CompatEntry));
> +            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), idstr);
> +            se->compat->instance_id = instance_id == -1 ?
> +                         calculate_compat_instance_id(idstr) : instance_id;
> +            instance_id = -1;
> +        }
> +    }
> +    pstrcat(se->idstr, sizeof(se->idstr), idstr);
> +
> +    if (instance_id == -1) {
> +        se->instance_id = calculate_new_instance_id(se->idstr);
> +    } else {
> +        se->instance_id = instance_id;
> +    }
> +    assert(!se->compat || se->instance_id == 0);
> +}
> +
>  /* TODO: Individual devices generally have very little idea about the rest
>     of the system, so instance_id should be removed/replaced.
>     Meanwhile pass -1 as instance_id if you do not already have a clearly
> @@ -1107,28 +1134,8 @@ int register_savevm_live(DeviceState *dev,
>      se->vmsd = NULL;
>      se->no_migrate = 0;
>  
> -    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> -        if (id) {
> -            pstrcpy(se->idstr, sizeof(se->idstr), id);
> -            pstrcat(se->idstr, sizeof(se->idstr), "/");
> -            qemu_free(id);
> -
> -            se->compat = qemu_mallocz(sizeof(CompatEntry));
> -            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), idstr);
> -            se->compat->instance_id = instance_id == -1 ?
> -                         calculate_compat_instance_id(idstr) : instance_id;
> -            instance_id = -1;
> -        }
> -    }
> -    pstrcat(se->idstr, sizeof(se->idstr), idstr);
> +    setup_instance_id(dev, idstr, instance_id, se);
>  
> -    if (instance_id == -1) {
> -        se->instance_id = calculate_new_instance_id(se->idstr);
> -    } else {
> -        se->instance_id = instance_id;
> -    }
> -    assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
>      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
>      return 0;
> @@ -1217,28 +1224,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>      se->vmsd = vmsd;
>      se->alias_id = alias_id;
>  
> -    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> -        if (id) {
> -            pstrcpy(se->idstr, sizeof(se->idstr), id);
> -            pstrcat(se->idstr, sizeof(se->idstr), "/");
> -            qemu_free(id);
> -
> -            se->compat = qemu_mallocz(sizeof(CompatEntry));
> -            pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), vmsd->name);
> -            se->compat->instance_id = instance_id == -1 ?
> -                         calculate_compat_instance_id(vmsd->name) : instance_id;
> -            instance_id = -1;
> -        }
> -    }
> -    pstrcat(se->idstr, sizeof(se->idstr), vmsd->name);
> +    setup_instance_id(dev, vmsd->name, instance_id, se);
>  
> -    if (instance_id == -1) {
> -        se->instance_id = calculate_new_instance_id(se->idstr);
> -    } else {
> -        se->instance_id = instance_id;
> -    }
> -    assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
>      QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
>      return 0;
> --
> 			Gleb.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-11-10 16:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-07 12:16 [Qemu-devel] [PATCH] Remove code duplication from savevm instance_id creation code Gleb Natapov
2010-11-10 16:01 ` [Qemu-devel] " Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).