Skip to contentSkip to conversation

libxl: rationalise libxl_device_* APIs

PATCH Superseded Ian Campbell 3 participants 39 messages Oct 13, 2011 – Oct 18, 2011 Arm/actions semi architecture

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 libxl: rationalise libxl_device_* APIs

The following series overhauls the libxl_device_* APIs in an attempt to make them more consistent across the types of devices and move towards something we can support as a stable API longterm.

This is broadly speaking the changes I discussed in [0]

One of the early patches adds a big comment describing the API. It would be useful if this got a particularly close eye with a view to supporting it long term -- especially from actual and potential consumers of the API (of who I've CC a few who sprang to mind).

Along the way I filed some rough edges of the internal implementation of this stuff but my primary concern is to make the public facing API one that we can commit to keeping stable.

One aspect which is missing is the ability to do asynchronous add/remove etc. This requires the overhaul of the libxl event system which Ian Jackson described at [1]. I did bear this in mind so hopefully I have provided the majority of the necessary moving parts internally.

Changes since last time:
- Rename type "destructor" functions to "dispose" freeing up the verb "destroy" to mean, uh, destroying things, such as a forcible remove of a device or destroying a domain.
- Use this new found freedom to s/force_remove/destroy/.
- Added a flags parameter to libxl_ctx_alloc. Might as well have one for future flexibility
- Updated language bindings to use new scheme.

[0] http://www.gossamer-threads.com/lists/xen/devel/204668
[1] http://www.gossamer-threads.com/lists/xen/devel/212580

 &  http://www.gossamer-threads.com/lists/xen/devel/212578
 &  http://www.gossamer-threads.com/lists/xen/devel/212579
 &  http://www.gossamer-threads.com/lists/xen/devel/212581
#1 lore raw

PATCH v2 1/1 libxl: add missing "break; " to do_pci_remove

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499238 -3600
# Node ID 1d8b47158206a15c035e794f6bc28e9e7066a2c0 # Parent 6ca28d551fd5f3d7d7849b6b95d3beda5b9cd10e
libxl: add missing "break;" to do_pci_remove

Otherwise we erroneously fall through the LIBXL_DOMAIN_TYPE_PV case into the "default: abort()".

(I'm sure we fixed this once already...)

tools/libxl/libxl_pci.c +1 -0
diff -r 6ca28d551fd5 -r 1d8b47158206 tools/libxl/libxl_pci.c
@@ -930,6 +930,7 @@ skip1:
             }
         }
         fclose(f);
+        break;
     }
     default:
         abort();
#2 lore raw

PATCH v2 1/1 libxl: remove generated file testidl.c on clean

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499238 -3600
# Node ID ed9950395a62c6077abf71a2e6d2a6502e0c59b4 # Parent 1d8b47158206a15c035e794f6bc28e9e7066a2c0
libxl: remove generated file testidl.c on clean

tools/libxl/Makefile +1 -0
diff -r 1d8b47158206 -r ed9950395a62 tools/libxl/Makefile
@@ -150,6 +150,7 @@ install: all
 clean:
 	$(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS)
 	$(RM) -f _*.c *.pyc _libxl_paths.*.tmp
+	$(RM) -f testidl.c.new testidl.c
 #	$(RM) -f $(AUTOSRCS) $(AUTOINCS)
 
 distclean: clean
#3 lore raw

PATCH v2 1/1 libxl: idl: use "dispose" rather than "destroy" for function to free IDL types

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499238 -3600
# Node ID 6bc67c8ad63e80f40003a2e3564e97e2f14942ef
# Parent ed9950395a62c6077abf71a2e6d2a6502e0c59b4
libxl: idl: use "dispose" rather than "destroy" for function to free IDL types

Destroy is an overloaded term which would commonly like to be used for actual destructive operations, such as destroying a domain etc.

Dispose isn't a great term but it does the job.

tools/libxl/gentest.py +2 -2
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/gentest.py
@@ -256,8 +256,8 @@ int main(int argc, char **argv)
         f.write("    printf(\"%%s: %%s\\n\", \"%s\", s);\n" % ty.typename)
         f.write("    if (s == NULL) abort();\n")
         f.write("    free(s);\n")
-        if ty.destructor_fn is not None:
-            f.write("    %s(&%s_val);\n" % (ty.destructor_fn, ty.typename))
+        if ty.dispose_fn is not None:
+            f.write("    %s(&%s_val);\n" % (ty.dispose_fn, ty.typename))
         f.write("\n")
 
     f.write("    printf(\"Testing Enumerations\\n\");\n")
tools/libxl/gentypes.py +11 -11
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/gentypes.py
@@ -74,7 +74,7 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
-def libxl_C_type_destroy(ty, v, indent = "    ", parent = None):
+def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
     s = ""
     if isinstance(ty, libxltypes.KeyedUnion):
         if parent is None:
@@ -83,16 +83,16 @@ def libxl_C_type_destroy(ty, v, indent =
         for f in ty.fields:
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += "case %s:\n" % f.enumname
-            s += libxl_C_type_destroy(f.type, fexpr, indent + "    ", nparent)
+            s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
             s += "    break;\n"
         s += "}\n"
-    elif isinstance(ty, libxltypes.Struct) and (parent is None or ty.destructor_fn is None):
+    elif isinstance(ty, libxltypes.Struct) and (parent is None or ty.dispose_fn is None):
         for f in [f for f in ty.fields if not f.const]:
             (nparent,fexpr) = ty.member(v, f, parent is None)
-            s += libxl_C_type_destroy(f.type, fexpr, "", nparent)
+            s += libxl_C_type_dispose(f.type, fexpr, "", nparent)
     else:
-        if ty.destructor_fn is not None:
-            s += "%s(%s);\n" % (ty.destructor_fn, ty.pass_arg(v, parent is None))
+        if ty.dispose_fn is not None:
+            s += "%s(%s);\n" % (ty.dispose_fn, ty.pass_arg(v, parent is None))
 
     if s != "":
         s = indent + s
@@ -217,8 +217,8 @@ if __name__ == '__main__':
 
     for ty in types:
         f.write(libxl_C_type_define(ty) + ";\n")
-        if ty.destructor_fn is not None:
-            f.write("void %s(%s);\n" % (ty.destructor_fn, ty.make_arg("p")))
+        if ty.dispose_fn is not None:
+            f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
         if ty.json_fn is not None:
             f.write("char *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.typename, ty.make_arg("p")))
         if isinstance(ty, libxltypes.Enumeration):
@@ -277,10 +277,10 @@ if __name__ == '__main__':
 
 """ % " ".join(sys.argv))
 
-    for ty in [t for t in types if t.destructor_fn is not None and t.autogenerate_destructor]:
-        f.write("void %s(%s)\n" % (ty.destructor_fn, ty.make_arg("p")))
+    for ty in [t for t in types if t.dispose_fn is not None and t.autogenerate_dispose_fn]:
+        f.write("void %s(%s)\n" % (ty.dispose_fn, ty.make_arg("p")))
         f.write("{\n")
-        f.write(libxl_C_type_destroy(ty, "p"))
+        f.write(libxl_C_type_dispose(ty, "p"))
         f.write("    memset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
         f.write("}\n")
         f.write("\n")
tools/libxl/idl.txt +3 -3
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/idl.txt
@@ -39,14 +39,14 @@ Type.passby: (default: libxltypes.PASS_B
    libxltypes.PASS_BY_VALUE
    libxltypes.PASS_BY_REFERENCE
 
-Type.destructor_fn: (default: typename + "_destroy" or None if type == None)
+Type.dispose_fn: (default: typename + "_dispose" or None if type == None)
 
  The name of the C function which will free all dynamically allocated
  memory contained within this type (but not the type itself).
 
-Type.autogenerate_destructor: (default: True)
+Type.autogenerate_dispose_fn: (default: True)
 
- Indicates if the above named Type.destructor_fn should be
+ Indicates if the above named Type.dispose_fn should be
  autogenerated.
 
 Type.json_fn: (default: typename + "_gen_json" or None if type == None)
tools/libxl/libxl.c +12 -12
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl.c
@@ -87,12 +87,12 @@ int libxl_ctx_free(libxl_ctx *ctx)
 {
     if (!ctx) return 0;
     if (ctx->xch) xc_interface_close(ctx->xch);
-    libxl_version_info_destroy(&ctx->version_info);
+    libxl_version_info_dispose(&ctx->version_info);
     if (ctx->xsh) xs_daemon_close(ctx->xsh);
     return 0;
 }
 
-void libxl_string_list_destroy(libxl_string_list *psl)
+void libxl_string_list_dispose(libxl_string_list *psl)
 {
     int i;
     libxl_string_list sl = *psl;
@@ -105,7 +105,7 @@ void libxl_string_list_destroy(libxl_str
     free(sl);
 }
 
-void libxl_key_value_list_destroy(libxl_key_value_list *pkvl)
+void libxl_key_value_list_dispose(libxl_key_value_list *pkvl)
 {
     int i;
     libxl_key_value_list kvl = *pkvl;
@@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
         libxl__qmp_cleanup(&gc, domid);
     }
     if (libxl__devices_destroy(&gc, domid, force) < 0)
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl_destroy_devices failed for %d", domid);
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl_devices_dispose failed for %d", domid);
 
     vm_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/vm", dom_path));
     if (vm_path)
@@ -1641,7 +1641,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     }
 out:
     for (i = 0; i < num; i++)
-        libxl_device_disk_destroy(&disks[i]);
+        libxl_device_disk_dispose(&disks[i]);
     free(disks);
     return ret;
 }
@@ -2241,7 +2241,7 @@ fail:
     xc_hypercall_buffer_free(ctx->xch, coremap);
     xc_hypercall_buffer_free(ctx->xch, socketmap);
     xc_hypercall_buffer_free(ctx->xch, nodemap);
-    libxl_topologyinfo_destroy(info);
+    libxl_topologyinfo_dispose(info);
     return ERROR_FAIL;
 }
 
@@ -2722,7 +2722,7 @@ int libxl_tmem_freeable(libxl_ctx *ctx)
     return rc;
 }
 
-void libxl_file_reference_destroy(libxl_file_reference *f)
+void libxl_file_reference_dispose(libxl_file_reference *f)
 {
     libxl__file_reference_unmap(f);
     free(f->path);
@@ -2852,7 +2852,7 @@ int libxl_destroy_cpupool(libxl_ctx *ctx
     rc = 0;
 
 out1:
-    libxl_cpumap_destroy(&cpumap);
+    libxl_cpumap_dispose(&cpumap);
 out:
     xc_cpupool_infofree(ctx->xch, info);
     libxl__free_all(&gc);
@@ -2941,10 +2941,10 @@ int libxl_cpupool_cpuadd_node(libxl_ctx 
         }
     }
 
-    libxl_topologyinfo_destroy(&topology);
+    libxl_topologyinfo_dispose(&topology);
 
 out:
-    libxl_cpumap_destroy(&freemap);
+    libxl_cpumap_dispose(&freemap);
     return rc;
 }
 
@@ -2993,11 +2993,11 @@ int libxl_cpupool_cpuremove_node(libxl_c
         }
     }
 
-    libxl_topologyinfo_destroy(&topology);
+    libxl_topologyinfo_dispose(&topology);
 
 out:
     for (p = 0; p < n_pools; p++) {
-        libxl_cpupoolinfo_destroy(poolinfo + p);
+        libxl_cpupoolinfo_dispose(poolinfo + p);
     }
 
     return ret;
tools/libxl/libxl.h +7 -7
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl.h
@@ -144,10 +144,10 @@ typedef uint8_t libxl_mac[6];
 #define LIBXL_MAC_BYTES(mac) mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]
 
 typedef char **libxl_string_list;
-void libxl_string_list_destroy(libxl_string_list *sl);
+void libxl_string_list_dispose(libxl_string_list *sl);
 
 typedef char **libxl_key_value_list;
-void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
+void libxl_key_value_list_dispose(libxl_key_value_list *kvl);
 
 typedef uint32_t libxl_hwcap[8];
 
@@ -155,14 +155,14 @@ typedef struct {
     uint32_t size;          /* number of bytes in map */
     uint8_t *map;
 } libxl_cpumap;
-void libxl_cpumap_destroy(libxl_cpumap *map);
+void libxl_cpumap_dispose(libxl_cpumap *map);
 
 typedef struct {
     uint32_t entries;
     uint32_t *array;
 } libxl_cpuarray;
 #define LIBXL_CPUARRAY_INVALID_ENTRY  ~0
-void libxl_cpuarray_destroy(libxl_cpuarray *array);
+void libxl_cpuarray_dispose(libxl_cpuarray *array);
 
 typedef struct {
     /*
@@ -174,7 +174,7 @@ typedef struct {
     void * data;
     size_t size;
 } libxl_file_reference;
-void libxl_file_reference_destroy(libxl_file_reference *p);
+void libxl_file_reference_dispose(libxl_file_reference *p);
 
 /* libxl_cpuid_policy_list is a dynamic array storing CPUID policies
  * for multiple leafs. It is terminated with an entry holding
@@ -182,7 +182,7 @@ void libxl_file_reference_destroy(libxl_
  */
 typedef struct libxl__cpuid_policy libxl_cpuid_policy;
 typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
-void libxl_cpuid_destroy(libxl_cpuid_policy_list *cpuid_list);
+void libxl_cpuid_dispose(libxl_cpuid_policy_list *cpuid_list);
 
 #define LIBXL_PCI_FUNC_ALL (~0U)
 
@@ -261,7 +261,7 @@ int libxl_init_dm_info(libxl_ctx *ctx,
 typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
-void libxl_domain_config_destroy(libxl_domain_config *d_config);
+void libxl_domain_config_dispose(libxl_domain_config *d_config);
 int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
                           uint32_t domid, int fd);
 int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
tools/libxl/libxl_cpuid.c +1 -1
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl_cpuid.c
@@ -16,7 +16,7 @@
 #include "libxl_osdeps.h"
 #include "libxl_internal.h"
 
-void libxl_cpuid_destroy(libxl_cpuid_policy_list *p_cpuid_list)
+void libxl_cpuid_dispose(libxl_cpuid_policy_list *p_cpuid_list)
 {
     int i, j;
     libxl_cpuid_policy_list cpuid_list = *p_cpuid_list;
tools/libxl/libxl_create.c +11 -11
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl_create.c
@@ -31,33 +31,33 @@
 #include "libxl_internal.h"
 #include "flexarray.h"
 
-void libxl_domain_config_destroy(libxl_domain_config *d_config)
+void libxl_domain_config_dispose(libxl_domain_config *d_config)
 {
     int i;
 
     for (i=0; i<d_config->num_disks; i++)
-        libxl_device_disk_destroy(&d_config->disks[i]);
+        libxl_device_disk_dispose(&d_config->disks[i]);
     free(d_config->disks);
 
     for (i=0; i<d_config->num_vifs; i++)
-        libxl_device_nic_destroy(&d_config->vifs[i]);
+        libxl_device_nic_dispose(&d_config->vifs[i]);
     free(d_config->vifs);
 
     for (i=0; i<d_config->num_pcidevs; i++)
-        libxl_device_pci_destroy(&d_config->pcidevs[i]);
+        libxl_device_pci_dispose(&d_config->pcidevs[i]);
     free(d_config->pcidevs);
 
     for (i=0; i<d_config->num_vfbs; i++)
-        libxl_device_vfb_destroy(&d_config->vfbs[i]);
+        libxl_device_vfb_dispose(&d_config->vfbs[i]);
     free(d_config->vfbs);
 
     for (i=0; i<d_config->num_vkbs; i++)
-        libxl_device_vkb_destroy(&d_config->vkbs[i]);
+        libxl_device_vkb_dispose(&d_config->vkbs[i]);
     free(d_config->vkbs);
 
-    libxl_domain_create_info_destroy(&d_config->c_info);
-    libxl_domain_build_info_destroy(&d_config->b_info);
-    libxl_device_model_info_destroy(&d_config->dm_info);
+    libxl_domain_create_info_dispose(&d_config->c_info);
+    libxl_domain_build_info_dispose(&d_config->b_info);
+    libxl_device_model_info_dispose(&d_config->dm_info);
 }
 
 int libxl_init_create_info(libxl_ctx *ctx, libxl_domain_create_info *c_info)
@@ -512,7 +512,7 @@ static int do_domain_create(libxl__gc *g
         if ( ret )
             goto error_out;
         libxl__device_console_add(gc, domid, &console, &state);
-        libxl_device_console_destroy(&console);
+        libxl_device_console_dispose(&console);
 
         dm_info->domid = domid;
         ret = libxl__create_device_model(gc, dm_info,
@@ -549,7 +549,7 @@ static int do_domain_create(libxl__gc *g
              console.consback = LIBXL_CONSOLE_BACKEND_IOEMU;
 
         libxl__device_console_add(gc, domid, &console, &state);
-        libxl_device_console_destroy(&console);
+        libxl_device_console_dispose(&console);
 
         if (need_qemu) {
             /* only copy those useful configs */
tools/libxl/libxl_types.idl +10 -10
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl_types.idl
@@ -8,13 +8,13 @@ namespace("libxl_")
 libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False)
 libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
 libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
-libxl_cpumap = Builtin("cpumap", destructor_fn="libxl_cpumap_destroy", passby=PASS_BY_REFERENCE)
-libxl_cpuarray = Builtin("cpuarray", destructor_fn="libxl_cpuarray_destroy", passby=PASS_BY_REFERENCE)
-libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)
+libxl_cpumap = Builtin("cpumap", dispose_fn="libxl_cpumap_dispose", passby=PASS_BY_REFERENCE)
+libxl_cpuarray = Builtin("cpuarray", dispose_fn="libxl_cpuarray_dispose", passby=PASS_BY_REFERENCE)
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
 
-libxl_string_list = Builtin("string_list", destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
-libxl_key_value_list = Builtin("key_value_list", destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
-libxl_file_reference = Builtin("file_reference", destructor_fn="libxl_file_reference_destroy", passby=PASS_BY_REFERENCE)
+libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_file_reference = Builtin("file_reference", dispose_fn="libxl_file_reference_dispose", passby=PASS_BY_REFERENCE)
 
 libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
 
@@ -109,7 +109,7 @@ SHUTDOWN_* constant."""),
     ("cpu_time",    uint64),
     ("vcpu_max_id", uint32),
     ("vcpu_online", uint32),
-    ], destructor_fn=None)
+    ], dispose_fn=None)
 
 libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("poolid",      uint32),
@@ -121,7 +121,7 @@ libxl_cpupoolinfo = Struct("cpupoolinfo"
 libxl_vminfo = Struct("vminfo", [
     ("uuid", libxl_uuid),
     ("domid", libxl_domid),
-    ], destructor_fn=None)
+    ], dispose_fn=None)
 
 libxl_version_info = Struct("version_info", [
     ("xen_version_major", integer),
@@ -362,7 +362,7 @@ libxl_physinfo = Struct("physinfo", [
     ("nr_nodes", uint32),
     ("hw_cap", libxl_hwcap),
     ("phys_cap", uint32),
-    ], destructor_fn=None, dir=DIR_OUT)
+    ], dispose_fn=None, dir=DIR_OUT)
 
 libxl_topologyinfo = Struct("topologyinfo", [
     ("coremap", libxl_cpuarray,   False, "cpu to core map"),
@@ -373,4 +373,4 @@ libxl_topologyinfo = Struct("topologyinf
 libxl_sched_credit = Struct("sched_credit", [
     ("weight", integer),
     ("cap", integer),
-    ], destructor_fn=None)
+    ], dispose_fn=None)
tools/libxl/libxl_utils.c +4 -4
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxl_utils.c
@@ -157,7 +157,7 @@ int libxl_name_to_cpupoolid(libxl_ctx *c
             }
             free(poolname);
         }
-        libxl_cpupoolinfo_destroy(poolinfo + i);
+        libxl_cpupoolinfo_dispose(poolinfo + i);
     }
     free(poolinfo);
     return ret;
@@ -478,7 +478,7 @@ int libxl_mac_to_device_nic(libxl_ctx *c
     }
 
     for (i=0; i<nb; i++)
-        libxl_nicinfo_destroy(&nics[i]);
+        libxl_nicinfo_dispose(&nics[i]);
     free(nics);
     return rc;
 }
@@ -579,7 +579,7 @@ int libxl_cpumap_alloc(libxl_ctx *ctx, l
     return 0;
 }
 
-void libxl_cpumap_destroy(libxl_cpumap *map)
+void libxl_cpumap_dispose(libxl_cpumap *map)
 {
     free(map->map);
 }
@@ -624,7 +624,7 @@ int libxl_cpuarray_alloc(libxl_ctx *ctx,
     return 0;
 }
 
-void libxl_cpuarray_destroy(libxl_cpuarray *array)
+void libxl_cpuarray_dispose(libxl_cpuarray *array)
 {
     free(array->array);
 }
tools/libxl/libxltypes.py +10 -10
diff -r ed9950395a62 -r 6bc67c8ad63e tools/libxl/libxltypes.py
@@ -44,11 +44,11 @@ class Type(object):
             self.rawname = typename
 
         if self.typename is not None:
-            self.destructor_fn = kwargs.setdefault('destructor_fn', self.typename + "_destroy")
+            self.dispose_fn = kwargs.setdefault('dispose_fn', self.typename + "_dispose")
         else:
-            self.destructor_fn = kwargs.setdefault('destructor_fn', None)
+            self.dispose_fn = kwargs.setdefault('dispose_fn', None)
 
-        self.autogenerate_destructor = kwargs.setdefault('autogenerate_destructor', True)
+        self.autogenerate_dispose_fn = kwargs.setdefault('autogenerate_dispose_fn', True)
 
         if self.typename is not None:
             self.json_fn = kwargs.setdefault('json_fn', self.typename + "_gen_json")
@@ -88,15 +88,15 @@ class Type(object):
 class Builtin(Type):
     """Builtin type"""
     def __init__(self, typename, **kwargs):
-        kwargs.setdefault('destructor_fn', None)
-        kwargs.setdefault('autogenerate_destructor', False)
+        kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('autogenerate_dispose_fn', False)
         kwargs.setdefault('autogenerate_json', False)
         Type.__init__(self, typename, **kwargs)
 
 class Number(Builtin):
     def __init__(self, ctype, **kwargs):
         kwargs.setdefault('namespace', None)
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('signed', False)
         kwargs.setdefault('json_fn', "yajl_gen_integer")
         self.signed = kwargs['signed']
@@ -105,7 +105,7 @@ class Number(Builtin):
 class UInt(Number):
     def __init__(self, w, **kwargs):
         kwargs.setdefault('namespace', None)
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         Number.__init__(self, "uint%d_t" % w, **kwargs)
 
         self.width = w
@@ -122,7 +122,7 @@ class EnumerationValue(object):
 
 class Enumeration(Type):
     def __init__(self, typename, values, **kwargs):
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         Type.__init__(self, typename, **kwargs)
 
         self.values = []
@@ -205,7 +205,7 @@ class Union(Aggregate):
         # union therefore any specific instance of this class will
         # need to provide an explicit destructor function.
         kwargs.setdefault('passby', PASS_BY_REFERENCE)
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         Aggregate.__init__(self, "union", name, fields, **kwargs)
 
 class KeyedUnion(Aggregate):
@@ -244,7 +244,7 @@ uint16 = UInt(16)
 uint32 = UInt(32)
 uint64 = UInt(64)
 
-string = Builtin("char *", namespace = None, destructor_fn = "free",
+string = Builtin("char *", namespace = None, dispose_fn = "free",
                  json_fn = "libxl__string_gen_json",
                  autogenerate_json = False)
 
tools/libxl/xl_cmdimpl.c +21 -21

157 lines not on this page · raw message

tools/python/genwrap.py +2 -2
diff -r ed9950395a62 -r 6bc67c8ad63e tools/python/genwrap.py
@@ -100,8 +100,8 @@ def py_attrib_set(ty, f):
 
 def py_object_def(ty):
     l = []
-    if ty.destructor_fn is not None:
-        dtor = '    %s(&self->obj);\n'%ty.destructor_fn
+    if ty.dispose_fn is not None:
+        dtor = '    %s(&self->obj);\n'%ty.dispose_fn
     else:
         dtor = ''
 
tools/python/xen/lowlevel/xl/xl.c +1 -1
diff -r ed9950395a62 -r 6bc67c8ad63e tools/python/xen/lowlevel/xl/xl.c
@@ -246,7 +246,7 @@ int attrib__libxl_hwcap_set(PyObject *v,
 int attrib__libxl_key_value_list_set(PyObject *v, libxl_key_value_list *pptr)
 {
     if ( *pptr ) {
-        libxl_key_value_list_destroy(pptr);
+        libxl_key_value_list_dispose(pptr);
         *pptr = NULL;
     }
     if ( v == Py_None )
#7 lore raw

PATCH v2 1/1 libxl: libxl_destroy_cpumap becomes libxl_cpumap_destroy

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499238 -3600
# Node ID a09c576e4445f8071e681b19ed531f24ea7ec095 # Parent 6bc67c8ad63e80f40003a2e3564e97e2f14942ef
libxl: libxl_destroy_cpumap becomes libxl_cpumap_destroy

Now that IDL type destructors are no longer libxl_*_destroy we can use the more common libxl_NOUN_VERB structure here.

tools/libxl/libxl.c +2 -2
diff -r 6bc67c8ad63e -r a09c576e4445 tools/libxl/libxl.c
@@ -2775,7 +2775,7 @@ int libxl_create_cpupool(libxl_ctx *ctx,
             if (rc) {
                 LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
                     "Error moving cpu to cpupool");
-                libxl_destroy_cpupool(ctx, *poolid);
+                libxl_cpupool_destroy(ctx, *poolid);
                 libxl__free_all(&gc);
                 return ERROR_FAIL;
             }
@@ -2799,7 +2799,7 @@ int libxl_create_cpupool(libxl_ctx *ctx,
     }
 }
 
-int libxl_destroy_cpupool(libxl_ctx *ctx, uint32_t poolid)
+int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     int rc, i;
tools/libxl/libxl.h +1 -1
diff -r 6bc67c8ad63e -r a09c576e4445 tools/libxl/libxl.h
@@ -503,7 +503,7 @@ int libxl_get_freecpus(libxl_ctx *ctx, l
 int libxl_create_cpupool(libxl_ctx *ctx, const char *name, int schedid,
                          libxl_cpumap cpumap, libxl_uuid *uuid,
                          uint32_t *poolid);
-int libxl_destroy_cpupool(libxl_ctx *ctx, uint32_t poolid);
+int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
 int libxl_cpupool_rename(libxl_ctx *ctx, const char *name, uint32_t poolid);
 int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu);
 int libxl_cpupool_cpuadd_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
tools/libxl/xl_cmdimpl.c +1 -1
diff -r 6bc67c8ad63e -r a09c576e4445 tools/libxl/xl_cmdimpl.c
@@ -4954,7 +4954,7 @@ int main_cpupooldestroy(int argc, char *
         return -ERROR_FAIL;
     }
 
-    return -libxl_destroy_cpupool(ctx, poolid);
+    return -libxl_cpupool_destroy(ctx, poolid);
 }
 
 int main_cpupoolrename(int argc, char **argv)
#8 lore raw

PATCH v2 1/1 libxl: add a comment describing the device interfaces

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499553 -3600
# Node ID 3ebd2ec3ff8054a1b7b0995dc0b79a16a866a96f # Parent a09c576e4445f8071e681b19ed531f24ea7ec095
libxl: add a comment describing the device interfaces.

Subsequent patches will endevour to make reality match this defined interface.

tools/libxl/libxl.h +59 -0
diff -r a09c576e4445 -r 3ebd2ec3ff80 tools/libxl/libxl.h
@@ -379,6 +379,65 @@ libxl_dominfo * libxl_list_domain(libxl_
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
 libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
 
+/*
+ * Devices
+ * =======
+ *
+ * Each device is represented by a libxl_device_<TYPE> data structure
+ * which is defined via the IDL. In addition some devices have an
+ * additional data type libxl_device_<TYPE>_getinfo which contains
+ * further runtime information about the device.
+ *
+ * A common set of methods are available for each device type. These
+ * are described below.
+ *
+ * Querying
+ * --------
+ *
+ * libxl_device_<type>_list(ctx, domid, nr):
+ *
+ *   Returns an array of libxl_device_<type> length nr representing
+ *   the devices attached to the specified domain.
+ *
+ * libxl_device_<type>_getinfo(ctx, domid, device, info):
+ *
+ *   Initialises info with details of the given device which must be
+ *   attached to the specified domain.
+ *
+ * Creation / Control
+ * ------------------
+ *
+ * libxl_device_<type>_init(ctx, device):
+ *
+ *    Initalises device to a default configuration.
+ *
+ * libxl_device_<type>_add(ctx, domid, device):
+ *
+ *   Adds the given device to the specified domain. This can be called
+ *   while the guest is running (hotplug) or before boot (coldplug).
+ *
+ *   This function only sets up the device but does not wait for the
+ *   domain to connect to the device and therefore cannot block on the
+ *   guest.
+ *
+ * libxl_device_<type>_remove(ctx, domid, device):
+ *
+ *   Removes the given device from the specified domain by performing
+ *   an orderly unplug with guest co-operation. This requires that the
+ *   guest is running.
+ *
+ *   This method is currently synchronous and therefore can block
+ *   while interacting with the guest.
+ *
+ * libxl_device_<type>_destroy(ctx, domid, device):
+ *
+ *   Removes the given device from the specified domain without guest
+ *   co-operation. It is guest specific what affect this will have on
+ *   a running guest.
+ *
+ *   This function does not interact with the guest and therefore
+ *   cannot block on the guest.
+ */
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 int libxl_device_disk_del(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk, int wait);
 libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num);
#9 lore raw

PATCH v2 1/1 libxl: various fixes to libxl_device_disk_list (and internals)

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID cd3c10775e17705a736e8c0efd2b302ef4b6866d
# Parent 3ebd2ec3ff8054a1b7b0995dc0b79a16a866a96f
libxl: various fixes to libxl_device_disk_list (and internals)

- handle realloc errors
- remove redundancy of libxl__append_disk_list_of_type return value and ndisks paramter.

tools/libxl/libxl.c +33 -13
diff -r 3ebd2ec3ff80 -r cd3c10775e17 tools/libxl/libxl.c
@@ -1505,11 +1505,11 @@ int libxl_device_vkb_hard_shutdown(libxl
     return ERROR_NI;
 }
 
-static unsigned int libxl__append_disk_list_of_type(libxl__gc *gc,
-                                                    uint32_t domid,
-                                                    const char *type,
-                                                    libxl_device_disk **disks,
-                                                    unsigned int *ndisks)
+static int libxl__append_disk_list_of_type(libxl__gc *gc,
+                                           uint32_t domid,
+                                           const char *type,
+                                           libxl_device_disk **disks,
+                                           int *ndisks)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *be_path = NULL;
@@ -1523,11 +1523,16 @@ static unsigned int libxl__append_disk_l
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
     if (dir) {
         char *removable;
-        *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
+        libxl_device_disk *tmp;
+        tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
+        if (tmp == NULL)
+            return ERROR_NOMEM;
+        *disks = tmp;
         pdisk = *disks + *ndisks;
         *ndisks += n;
         pdisk_end = *disks + *ndisks;
         for (; pdisk < pdisk_end; pdisk++, dir++) {
+            memset(pdisk, 0, sizeof(*pdisk));
             pdisk->backend_domid = 0;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
@@ -1555,22 +1560,37 @@ static unsigned int libxl__append_disk_l
             pdisk->format = LIBXL_DISK_FORMAT_UNKNOWN;
         }
     }
-
-    return n;
+    return 0;
 }
 
 libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     libxl_device_disk *disks = NULL;
-    unsigned int ndisks = 0;
-
-    *num = libxl__append_disk_list_of_type(&gc, domid, "vbd", &disks, &ndisks);
-    *num += libxl__append_disk_list_of_type(&gc, domid, "tap", &disks, &ndisks);
-    *num += libxl__append_disk_list_of_type(&gc, domid, "qdisk", &disks, &ndisks);
+    int rc;
+
+    *num = 0;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "vbd", &disks, num);
+    if (rc) goto out_err;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "tap", &disks, num);
+    if (rc) goto out_err;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "qdisk", &disks, num);
+    if (rc) goto out_err;
 
     libxl__free_all(&gc);
     return disks;
+
+out_err:
+    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list disks");
+    while (disks && *num) {
+        (*num)--;
+        libxl_device_disk_dispose(&disks[*num]);
+    }
+    free(disks);
+    return NULL;
 }
 
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
#10 lore raw

PATCH v2 1/1 libxl: refactor code to construct disk from xenstore backend

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 1960247c5d873a4aede18c1aefcc28e78e6ea7dd # Parent cd3c10775e17705a736e8c0efd2b302ef4b6866d
libxl: refactor code to construct disk from xenstore backend

Temporarily retain unsafe behaviour of reading f.e. directory.

tools/libxl/libxl.c +52 -29
diff -r cd3c10775e17 -r 1960247c5d87 tools/libxl/libxl.c
@@ -1505,24 +1505,69 @@ int libxl_device_vkb_hard_shutdown(libxl
     return ERROR_NI;
 }
 
+static void libxl__device_disk_from_xs_be(libxl__gc *gc,
+                                          const char *be_path,
+                                          libxl_device_disk *disk)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    unsigned int len;
+    char *tmp;
+    const char *fe_path; /* XXX unsafe */
+
+    memset(disk, 0, sizeof(*disk));
+
+    tmp = xs_read(ctx->xsh, XBT_NULL,
+                  libxl__sprintf(gc, "%s/params", be_path), &len);
+    if (tmp && strchr(tmp, ':')) {
+        disk->pdev_path = strdup(strchr(tmp, ':') + 1);
+        free(tmp);
+    } else {
+        disk->pdev_path = tmp;
+    }
+    libxl_string_to_backend(ctx,
+                        libxl__xs_read(gc, XBT_NULL,
+                                       libxl__sprintf(gc, "%s/type", be_path)),
+                        &(disk->backend));
+    disk->vdev = xs_read(ctx->xsh, XBT_NULL,
+                         libxl__sprintf(gc, "%s/dev", be_path), &len);
+    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf
+                         (gc, "%s/removable", be_path));
+
+    if (tmp)
+        disk->removable = atoi(tmp);
+    else
+        disk->removable = 0;
+
+    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/mode", be_path));
+    if (!strcmp(tmp, "w"))
+        disk->readwrite = 1;
+    else
+        disk->readwrite = 0;
+
+    fe_path = libxl__xs_read(gc, XBT_NULL,
+                             libxl__sprintf(gc, "%s/frontend", be_path));
+    tmp = libxl__xs_read(gc, XBT_NULL,
+                         libxl__sprintf(gc, "%s/device-type", fe_path));
+    disk->is_cdrom = !strcmp(tmp, "cdrom");
+
+    disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
+}
+
 static int libxl__append_disk_list_of_type(libxl__gc *gc,
                                            uint32_t domid,
                                            const char *type,
                                            libxl_device_disk **disks,
                                            int *ndisks)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
     char *be_path = NULL;
     char **dir = NULL;
-    unsigned int n = 0, len = 0;
+    unsigned int n = 0;
     libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
-    char *physpath_tmp = NULL;
 
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
     if (dir) {
-        char *removable;
         libxl_device_disk *tmp;
         tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
         if (tmp == NULL)
@@ -1532,32 +1577,10 @@ static int libxl__append_disk_list_of_ty
         *ndisks += n;
         pdisk_end = *disks + *ndisks;
         for (; pdisk < pdisk_end; pdisk++, dir++) {
-            memset(pdisk, 0, sizeof(*pdisk));
+            const char *p;
+            p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
+            libxl__device_disk_from_xs_be(gc, p, pdisk);
             pdisk->backend_domid = 0;
-            physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/params", be_path, *dir), &len);
-            if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
-                free(physpath_tmp);
-            } else {
-                pdisk->pdev_path = physpath_tmp;
-            }
-            libxl_string_to_backend(ctx, libxl__xs_read(gc, XBT_NULL,
-                libxl__sprintf(gc, "%s/%s/type", be_path, *dir)),
-                &(pdisk->backend));
-            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(gc, "%s/%s/dev", be_path, *dir), &len);
-            removable = libxl__xs_read(gc, XBT_NULL, libxl__sprintf
-                                       (gc, "%s/%s/removable", be_path, *dir));
-            if (removable)
-                pdisk->removable = atoi(removable);
-            else
-                pdisk->removable = 0;
-            if (!strcmp(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/mode", be_path, *dir)), "w"))
-                pdisk->readwrite = 1;
-            else
-                pdisk->readwrite = 0;
-            type = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/device-type", libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s/frontend", be_path, *dir))));
-            pdisk->is_cdrom = !strcmp(type, "cdrom");
-            pdisk->format = LIBXL_DISK_FORMAT_UNKNOWN;
         }
     }
     return 0;
#11 lore raw

PATCH v2 1/1 libxl: do not read f.e. xenstore dir in disk list function

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID c6c7a393fe4a6223df6952cc5fed04379166dc38 # Parent 1960247c5d873a4aede18c1aefcc28e78e6ea7dd
libxl: do not read f.e. xenstore dir in disk list function

Instead store a duplicate of the "device-type" node in the backend dir and use that instead.

This maintains the invariant that the list function is always "safe".

XXX I'm not sure this is an actual issue. The entries in the frontend device XXX area are writeable by the guest but maybe the solution is to restrict that?

tools/libxl/libxl.c +3 -4
diff -r 1960247c5d87 -r c6c7a393fe4a tools/libxl/libxl.c
@@ -1018,6 +1018,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
     flexarray_append(back, "mode");
     flexarray_append(back, disk->readwrite ? "w" : "r");
+    flexarray_append(back, "device-type");
+    flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
 
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(&gc, "%d", disk->backend_domid));
@@ -1512,7 +1514,6 @@ static void libxl__device_disk_from_xs_b
     libxl_ctx *ctx = libxl__gc_owner(gc);
     unsigned int len;
     char *tmp;
-    const char *fe_path; /* XXX unsafe */
 
     memset(disk, 0, sizeof(*disk));
 
@@ -1544,10 +1545,8 @@ static void libxl__device_disk_from_xs_b
     else
         disk->readwrite = 0;
 
-    fe_path = libxl__xs_read(gc, XBT_NULL,
-                             libxl__sprintf(gc, "%s/frontend", be_path));
     tmp = libxl__xs_read(gc, XBT_NULL,
-                         libxl__sprintf(gc, "%s/device-type", fe_path));
+                         libxl__sprintf(gc, "%s/device-type", be_path));
     disk->is_cdrom = !strcmp(tmp, "cdrom");
 
     disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
#15 lore raw

PATCH v2 1/1 libxl: update nic list API to use common device API style

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 32a1ea4a0b13f2b9bdc9385cc175ed28086d5681 # Parent c6c7a393fe4a6223df6952cc5fed04379166dc38
libxl: update nic list API to use common device API style

libxl_device_nic_list returns an array of libxl_device_nic and libxl_device_nic_getinfo retrieves further information.

tools/libxl/libxl.c +125 -47
diff -r c6c7a393fe4a -r 32a1ea4a0b13 tools/libxl/libxl.c
@@ -1291,60 +1291,138 @@ int libxl_device_nic_del(libxl_ctx *ctx,
     return rc;
 }
 
-libxl_nicinfo *libxl_list_nics(libxl_ctx *ctx, uint32_t domid, unsigned int *nb)
+static void libxl__device_nic_from_xs_be(libxl__gc *gc,
+                                         const char *be_path,
+                                         libxl_device_nic *nic)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    unsigned int len;
+    char *tmp;
+    int rc;
+
+    memset(nic, 0, sizeof(*nic));
+
+    tmp = xs_read(ctx->xsh, XBT_NULL,
+                  libxl__sprintf(gc, "%s/handle", be_path), &len);
+    if ( tmp )
+        nic->devid = atoi(tmp);
+    else
+        nic->devid = 0;
+
+    /* nic->mtu = */
+
+    tmp = xs_read(ctx->xsh, XBT_NULL,
+                  libxl__sprintf(gc, "%s/mac", be_path), &len);
+    rc = libxl__parse_mac(tmp, nic->mac);
+    if (rc)
+            memset(nic->mac, 0, sizeof(nic->mac));
+
+    nic->ip = xs_read(ctx->xsh, XBT_NULL,
+                      libxl__sprintf(gc, "%s/ip", be_path), &len);
+
+    nic->bridge = xs_read(ctx->xsh, XBT_NULL,
+                      libxl__sprintf(gc, "%s/bridge", be_path), &len);
+
+    nic->script = xs_read(ctx->xsh, XBT_NULL,
+                      libxl__sprintf(gc, "%s/script", be_path), &len);
+
+    /* XXX ioemu nics are not in xenstore at all? */
+    nic->nictype = LIBXL_NIC_TYPE_VIF;
+    nic->model = NULL; /* XXX Only for TYPE_IOEMU */
+    nic->ifname = NULL; /* XXX Only for TYPE_IOEMU */
+}
+
+static int libxl__append_nic_list_of_type(libxl__gc *gc,
+                                           uint32_t domid,
+                                           const char *type,
+                                           libxl_device_nic **nics,
+                                           int *nnics)
+{
+    char *be_path = NULL;
+    char **dir = NULL;
+    unsigned int n = 0;
+    libxl_device_nic *pnic = NULL, *pnic_end = NULL;
+
+    be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
+                             libxl__xs_get_dompath(gc, 0), type, domid);
+    dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
+    if (dir) {
+        libxl_device_nic *tmp;
+        tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
+        if (tmp == NULL)
+            return ERROR_NOMEM;
+        *nics = tmp;
+        pnic = *nics + *nnics;
+        *nnics += n;
+        pnic_end = *nics + *nnics;
+        for (; pnic < pnic_end; pnic++, dir++) {
+            const char *p;
+            p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
+            libxl__device_nic_from_xs_be(gc, p, pnic);
+            pnic->backend_domid = 0;
+        }
+    }
+    return 0;
+}
+
+libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dompath, *nic_path_fe;
-    char **l, **list;
-    char *val, *tok;
-    unsigned int nb_nics, i;
-    libxl_nicinfo *res, *nics;
+    libxl_device_nic *nics = NULL;
+    int rc;
+
+    *num = 0;
+
+    rc = libxl__append_nic_list_of_type(&gc, domid, "vif", &nics, num);
+    if (rc) goto out_err;
+
+    libxl__free_all(&gc);
+    return nics;
+
+out_err:
+    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list nics");
+    while (*num) {
+        (*num)--;
+        libxl_device_nic_dispose(&nics[*num]);
+    }
+    free(nics);
+    return NULL;
+}
+
+int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_nic *nic, libxl_nicinfo *nicinfo)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *dompath, *nicpath;
+    char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    if (!dompath)
-        goto err;
-    list = l = libxl__xs_directory(&gc, XBT_NULL,
-                           libxl__sprintf(&gc, "%s/device/vif", dompath), &nb_nics);
-    if (!l)
-        goto err;
-    nics = res = calloc(nb_nics, sizeof (libxl_nicinfo));
-    if (!res)
-        goto err;
-    for (*nb = nb_nics; nb_nics > 0; --nb_nics, ++l, ++nics) {
-        nic_path_fe = libxl__sprintf(&gc, "%s/device/vif/%s", dompath, *l);
-
-        nics->backend = xs_read(ctx->xsh, XBT_NULL,
-                                libxl__sprintf(&gc, "%s/backend", nic_path_fe), NULL);
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend-id", nic_path_fe));
-        nics->backend_id = val ? strtoul(val, NULL, 10) : -1;
-
-        nics->devid = strtoul(*l, NULL, 10);
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/state", nic_path_fe));
-        nics->state = val ? strtoul(val, NULL, 10) : -1;
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
-        for (i = 0, tok = strtok(val, ":"); tok && (i < 6);
-             ++i, tok = strtok(NULL, ":")) {
-            nics->mac[i] = strtoul(tok, NULL, 16);
-        }
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/event-channel", nic_path_fe));
-        nics->evtch = val ? strtol(val, NULL, 10) : -1;
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/tx-ring-ref", nic_path_fe));
-        nics->rref_tx = val ? strtol(val, NULL, 10) : -1;
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/rx-ring-ref", nic_path_fe));
-        nics->rref_rx = val ? strtol(val, NULL, 10) : -1;
-        nics->frontend = xs_read(ctx->xsh, XBT_NULL,
-                                 libxl__sprintf(&gc, "%s/frontend", nics->backend), NULL);
-        val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/frontend-id", nics->backend));
-        nics->frontend_id = val ? strtoul(val, NULL, 10) : -1;
-        nics->script = xs_read(ctx->xsh, XBT_NULL,
-                               libxl__sprintf(&gc, "%s/script", nics->backend), NULL);
+    nicinfo->devid = nic->devid;
+
+    nicpath = libxl__sprintf(&gc, "%s/device/vif/%d", dompath, nicinfo->devid);
+    nicinfo->backend = xs_read(ctx->xsh, XBT_NULL,
+                                libxl__sprintf(&gc, "%s/backend", nicpath), NULL);
+    if (!nicinfo->backend) {
+        libxl__free_all(&gc);
+        return ERROR_FAIL;
     }
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend-id", nicpath));
+    nicinfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/state", nicpath));
+    nicinfo->state = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/event-channel", nicpath));
+    nicinfo->evtch = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/tx-ring-ref", nicpath));
+    nicinfo->rref_tx = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/rx-ring-ref", nicpath));
+    nicinfo->rref_rx = val ? strtoul(val, NULL, 10) : -1;
+    nicinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                 libxl__sprintf(&gc, "%s/frontend", nicinfo->backend), NULL);
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/frontend-id", nicinfo->backend));
+    nicinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
 
     libxl__free_all(&gc);
-    return res;
-err:
-    libxl__free_all(&gc);
-    return NULL;
+    return 0;
 }
 
 /******************************************************************************/
tools/libxl/libxl.h +3 -1
diff -r c6c7a393fe4a -r 32a1ea4a0b13 tools/libxl/libxl.h
@@ -454,7 +454,9 @@ int libxl_device_disk_local_detach(libxl
 int libxl_device_nic_init(libxl_device_nic *nic, int dev_num);
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_del(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic, int wait);
-libxl_nicinfo *libxl_list_nics(libxl_ctx *ctx, uint32_t domid, unsigned int *nb);
+libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num);
+int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
 int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, libxl_device_console *console);
 
tools/libxl/libxl_types.idl +0 -2
diff -r c6c7a393fe4a -r 32a1ea4a0b13 tools/libxl/libxl_types.idl
@@ -330,8 +330,6 @@ libxl_nicinfo = Struct("nicinfo", [
     ("frontend_id", uint32),
     ("devid", integer),
     ("state", integer),
-    ("script", string),
-    ("mac", libxl_mac),
     ("evtch", integer),
     ("rref_tx", integer),
     ("rref_rx", integer),
tools/libxl/libxl_utils.c +9 -9
diff -r c6c7a393fe4a -r 32a1ea4a0b13 tools/libxl/libxl_utils.c
@@ -451,15 +451,15 @@ int libxl_pipe(libxl_ctx *ctx, int pipes
 int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
                             const char *mac, libxl_device_nic *nic)
 {
-    libxl_nicinfo *nics;
-    unsigned int nb, rc, i;
+    libxl_device_nic *nics;
+    int nb, rc, i;
     libxl_mac mac_n;
 
     rc = libxl__parse_mac(mac, mac_n);
     if (rc)
         return rc;
 
-    nics = libxl_list_nics(ctx, domid, &nb);
+    nics = libxl_device_nic_list(ctx, domid, &nb);
     if (!nics)
         return ERROR_FAIL;
 
@@ -468,17 +468,17 @@ int libxl_mac_to_device_nic(libxl_ctx *c
     rc = ERROR_INVAL;
     for (i = 0; i < nb; ++i) {
         if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
-            nic->backend_domid = nics[i].backend_id;
-            nic->devid = nics[i].devid;
-            memcpy(nic->mac, nics[i].mac, sizeof (nic->mac));
-            nic->script = strdup(nics[i].script);
+            *nic = nics[i];
             rc = 0;
+            i++; /* Do not dispose this NIC on exit path */
             break;
         }
+        libxl_device_nic_dispose(&nics[i]);
     }
 
-    for (i=0; i<nb; i++)
-        libxl_nicinfo_dispose(&nics[i]);
+    for (; i<nb; i++)
+        libxl_device_nic_dispose(&nics[i]);
+
     free(nics);
     return rc;
 }
tools/libxl/xl_cmdimpl.c +17 -12
diff -r c6c7a393fe4a -r 32a1ea4a0b13 tools/libxl/xl_cmdimpl.c
@@ -4025,8 +4025,9 @@ int main_networkattach(int argc, char **
 int main_networklist(int argc, char **argv)
 {
     int opt;
-    libxl_nicinfo *nics;
-    unsigned int nb, i;
+    libxl_device_nic *nics;
+    libxl_nicinfo nicinfo;
+    int nb, i;
 
     if ((opt = def_getopt(argc, argv, "", "network-list", 1)) != -1)
         return opt;
@@ -4039,19 +4040,23 @@ int main_networklist(int argc, char **ar
             fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             continue;
         }
-        if (!(nics = libxl_list_nics(ctx, domid, &nb))) {
+        nics = libxl_device_nic_list(ctx, domid, &nb);
+        if (!nics) {
             continue;
         }
         for (i = 0; i < nb; ++i) {
-            /* Idx BE */
-            printf("%-3d %-2d ", nics[i].devid, nics[i].backend_id);
-            /* MAC */
-            printf(LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
-            /* Hdl  Sta  evch txr/rxr  BE-path */
-            printf("%6d %5d %6d %5d/%-11d %-30s\n",
-                   nics[i].devid, nics[i].state, nics[i].evtch,
-                   nics[i].rref_tx, nics[i].rref_rx, nics[i].backend);
-            libxl_nicinfo_dispose(&nics[i]);
+            if (!libxl_device_nic_getinfo(ctx, domid, &nics[i], &nicinfo)) {
+                /* Idx BE */
+                printf("%-3d %-2d ", nicinfo.devid, nicinfo.backend_id);
+                /* MAC */
+                printf(LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
+                /* Hdl  Sta  evch txr/rxr  BE-path */
+                printf("%6d %5d %6d %5d/%-11d %-30s\n",
+                       nicinfo.devid, nicinfo.state, nicinfo.evtch,
+                       nicinfo.rref_tx, nicinfo.rref_rx, nicinfo.backend);
+                libxl_nicinfo_dispose(&nicinfo);
+            }
+            libxl_device_nic_dispose(&nics[i]);
         }
         free(nics);
     }
#16 lore raw

PATCH v2 1/1 libxl: reimplement devid->nic in terms of from_xs_be function

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 0ffea6ba20dddbf2a79cfed884a5444446c196ba
# Parent 32a1ea4a0b13f2b9bdc9385cc175ed28086d5681
libxl: reimplement devid->nic in terms of from_xs_be function.

tools/libxl/libxl.c +27 -1
diff -r 32a1ea4a0b13 -r 0ffea6ba20dd tools/libxl/libxl.c
@@ -1315,7 +1315,7 @@ static void libxl__device_nic_from_xs_be
                   libxl__sprintf(gc, "%s/mac", be_path), &len);
     rc = libxl__parse_mac(tmp, nic->mac);
     if (rc)
-            memset(nic->mac, 0, sizeof(nic->mac));
+        memset(nic->mac, 0, sizeof(nic->mac));
 
     nic->ip = xs_read(ctx->xsh, XBT_NULL,
                       libxl__sprintf(gc, "%s/ip", be_path), &len);
@@ -1332,6 +1332,32 @@ static void libxl__device_nic_from_xs_be
     nic->ifname = NULL; /* XXX Only for TYPE_IOEMU */
 }
 
+int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
+                              const char *devid, libxl_device_nic *nic)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *dompath, *path;
+    int rc = ERROR_FAIL;
+
+    memset(nic, 0, sizeof (libxl_device_nic));
+    dompath = libxl__xs_get_dompath(&gc, domid);
+    if (!dompath)
+        goto out;
+
+    path = libxl__xs_read(&gc, XBT_NULL,
+                          libxl__sprintf(&gc, "%s/device/vif/%s/backend",
+                                         dompath, devid));
+    if (!path)
+        goto out;
+
+    libxl__device_nic_from_xs_be(&gc, path, nic);
+
+    rc = 0;
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
 static int libxl__append_nic_list_of_type(libxl__gc *gc,
                                            uint32_t domid,
                                            const char *type,
tools/libxl/libxl_utils.c +0 -35
diff -r 32a1ea4a0b13 -r 0ffea6ba20dd tools/libxl/libxl_utils.c
@@ -483,41 +483,6 @@ int libxl_mac_to_device_nic(libxl_ctx *c
     return rc;
 }
 
-int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
-                              const char *devid, libxl_device_nic *nic)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *val;
-    char *dompath, *nic_path_fe, *nic_path_be;
-    int rc = ERROR_FAIL;
-
-    memset(nic, 0, sizeof (libxl_device_nic));
-    dompath = libxl__xs_get_dompath(&gc, domid);
-    if (!dompath) {
-        goto out;
-    }
-    nic_path_fe = libxl__sprintf(&gc, "%s/device/vif/%s", dompath, devid);
-    nic_path_be = libxl__xs_read(&gc, XBT_NULL,
-                                libxl__sprintf(&gc, "%s/backend", nic_path_fe));
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend-id", nic_path_fe));
-    if ( NULL == val ) {
-        goto out;
-    }
-    nic->backend_domid = strtoul(val, NULL, 10);
-    nic->devid = strtoul(devid, NULL, 10);
-
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mac", nic_path_fe));
-    rc = libxl__parse_mac(val, nic->mac);
-    if (rc)
-        goto out;
-
-    nic->script = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/script", nic_path_be), NULL);
-    rc = 0;
-out:
-    libxl__free_all(&gc);
-    return rc;
-}
-
 int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
                                const char *devid, libxl_device_disk *disk)
 {
#17 lore raw

PATCH v2 1/1 libxl: reimplement devid->disk in terms of from_xs_be function

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 521f9749c03271cc3de0b95e2bf1ea574080fd90
# Parent 0ffea6ba20dddbf2a79cfed884a5444446c196ba
libxl: reimplement devid->disk in terms of from_xs_be function.

tools/libxl/libxl.c +27 -0
diff -r 0ffea6ba20dd -r 521f9749c032 tools/libxl/libxl.c
@@ -1656,6 +1656,33 @@ static void libxl__device_disk_from_xs_b
     disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
 }
 
+int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
+                               const char *devid, libxl_device_disk *disk)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *dompath, *path;
+    int rc = ERROR_FAIL;
+
+    memset(disk, 0, sizeof (libxl_device_disk));
+    dompath = libxl__xs_get_dompath(&gc, domid);
+    if (!dompath) {
+        goto out;
+    }
+    path = libxl__xs_read(&gc, XBT_NULL,
+                          libxl__sprintf(&gc, "%s/device/vbd/%s/backend",
+                                         dompath, devid));
+    if (!path)
+        goto out;
+
+    libxl__device_disk_from_xs_be(&gc, path, disk);
+
+    rc = 0;
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
+
 static int libxl__append_disk_list_of_type(libxl__gc *gc,
                                            uint32_t domid,
                                            const char *type,
tools/libxl/libxl_utils.c +0 -44

50 lines not on this page · raw message

#18 lore raw

PATCH v2 1/1 libxl: libxl_devid_to_* should take an integer device id

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID b8b0e5d41f320124c13bd8d39627b9366ff270f1 # Parent 521f9749c03271cc3de0b95e2bf1ea574080fd90
libxl: libxl_devid_to_* should take an integer device id

Currently takes a string.

tools/libxl/libxl.c +4 -4
diff -r 521f9749c032 -r b8b0e5d41f32 tools/libxl/libxl.c
@@ -1333,7 +1333,7 @@ static void libxl__device_nic_from_xs_be
 }
 
 int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
-                              const char *devid, libxl_device_nic *nic)
+                              int devid, libxl_device_nic *nic)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     char *dompath, *path;
@@ -1345,7 +1345,7 @@ int libxl_devid_to_device_nic(libxl_ctx 
         goto out;
 
     path = libxl__xs_read(&gc, XBT_NULL,
-                          libxl__sprintf(&gc, "%s/device/vif/%s/backend",
+                          libxl__sprintf(&gc, "%s/device/vif/%d/backend",
                                          dompath, devid));
     if (!path)
         goto out;
@@ -1657,7 +1657,7 @@ static void libxl__device_disk_from_xs_b
 }
 
 int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
-                               const char *devid, libxl_device_disk *disk)
+                               int devid, libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     char *dompath, *path;
@@ -1669,7 +1669,7 @@ int libxl_devid_to_device_disk(libxl_ctx
         goto out;
     }
     path = libxl__xs_read(&gc, XBT_NULL,
-                          libxl__sprintf(&gc, "%s/device/vbd/%s/backend",
+                          libxl__sprintf(&gc, "%s/device/vbd/%d/backend",
                                          dompath, devid));
     if (!path)
         goto out;
tools/libxl/libxl_utils.h +4 -4
diff -r 521f9749c032 -r b8b0e5d41f32 tools/libxl/libxl_utils.h
@@ -60,11 +60,11 @@ void libxl_report_child_exitstatus(libxl
 
 int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
                             const char *mac, libxl_device_nic *nic);
-int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
-                              const char *devid, libxl_device_nic *nic);
+int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid, int devid,
+                              libxl_device_nic *nic);
 
-int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
-                               const char *devid, libxl_device_disk *disk);
+int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid, int devid,
+                               libxl_device_disk *disk);
 
 int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap);
 int libxl_cpumap_test(libxl_cpumap *cpumap, int cpu);
tools/libxl/xl_cmdimpl.c +2 -2
diff -r 521f9749c032 -r b8b0e5d41f32 tools/libxl/xl_cmdimpl.c
@@ -4077,7 +4077,7 @@ int main_networkdetach(int argc, char **
     }
 
     if (!strchr(argv[optind+1], ':')) {
-        if (libxl_devid_to_device_nic(ctx, domid, argv[optind+1], &nic)) {
+        if (libxl_devid_to_device_nic(ctx, domid, atoi(argv[optind+1]), &nic)) {
             fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
             return 1;
         }
@@ -4178,7 +4178,7 @@ int main_blockdetach(int argc, char **ar
         fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
-    if (libxl_devid_to_device_disk(ctx, domid, argv[optind+1], &disk)) {
+    if (libxl_devid_to_device_disk(ctx, domid, atoi(argv[optind+1]), &disk)) {
         fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
#19 lore raw

PATCH v2 1/1 libxl: separate forced and non-forced device remove

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499565 -3600
# Node ID 0584450afaf8c1236f0c0b870fc16629c1233662 # Parent b8b0e5d41f320124c13bd8d39627b9366ff270f1
libxl: separate forced and non-forced device remove.

The function libxl__device_destroy currently takes a force parameter however:

* in the forced case we initiate a graceful shutdown and then immediately nuke the backend directory, quite likely before anyone got a chance to react.
* the callers all have a "wait" variable and pass in "!wait" as the force argument which is confusing since not waiting is not really the same thing as forcing the destroy.

Therefore split the function into libxl__device_remove and libxl__device_destroy. The former initiates a graceful shutdown which the latter simply nukes the backend directory.

This makes some of the callers look a bit odd but that should fall out as I continue to pull this piece of string.

tools/libxl/libxl_device.c +22 -10
diff -r b8b0e5d41f32 -r 0584450afaf8 tools/libxl/libxl_device.c
@@ -365,7 +365,7 @@ int libxl__device_disk_dev_number(const 
     return -1;
 }
 
-int libxl__device_destroy(libxl__gc *gc, char *be_path, int force)
+int libxl__device_remove(libxl__gc *gc, char *be_path)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xs_transaction_t t;
@@ -393,17 +393,22 @@ retry_transaction:
             goto out;
         }
     }
-    if (!force) {
-        xs_watch(ctx->xsh, state_path, be_path);
-        rc = 1;
-    } else {
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
-    }
+
+    xs_watch(ctx->xsh, state_path, be_path);
     libxl__device_destroy_tapdisk(gc, be_path);
+    rc = 1;
 out:
     return rc;
 }
 
+int libxl__device_destroy(libxl__gc *gc, char *be_path)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    xs_rm(ctx->xsh, XBT_NULL, be_path);
+    libxl__device_destroy_tapdisk(gc, be_path);
+    return 0;
+}
+
 static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -461,7 +466,9 @@ int libxl__devices_destroy(libxl__gc *gc
             fe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", domid, l1[i], l2[j]);
             be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path));
             if (be_path != NULL) {
-                if (libxl__device_destroy(gc, be_path, force) > 0)
+                int rc = force ? libxl__device_destroy(gc, be_path)
+                               : libxl__device_remove(gc, be_path);
+                if (rc > 0)
                     n_watches++;
             } else {
                 xs_rm(ctx->xsh, XBT_NULL, path);
@@ -473,7 +480,9 @@ int libxl__devices_destroy(libxl__gc *gc
     fe_path = libxl__sprintf(gc, "/local/domain/%d/console", domid);
     be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path));
     if (be_path && strcmp(be_path, "")) {
-        if (libxl__device_destroy(gc, be_path, force) > 0)
+        int rc = force ? libxl__device_destroy(gc, be_path)
+                       : libxl__device_remove(gc, be_path);
+        if (rc > 0)
             n_watches++;
     }
 
@@ -506,7 +515,10 @@ int libxl__device_del(libxl__gc *gc, lib
 
     backend_path = libxl__device_backend_path(gc, dev);
 
-    rc = libxl__device_destroy(gc, backend_path, !wait);
+    if (wait)
+        rc = libxl__device_remove(gc, backend_path);
+    else
+        rc = libxl__device_destroy(gc, backend_path);
     if (rc == -1) {
         rc = ERROR_FAIL;
         goto out;
tools/libxl/libxl_internal.h +2 -1
diff -r b8b0e5d41f32 -r 0584450afaf8 tools/libxl/libxl_internal.h
@@ -253,7 +253,8 @@ _hidden int libxl__device_generic_add(li
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev, int wait);
-_hidden int libxl__device_destroy(libxl__gc *gc, char *be_path, int force);
+_hidden int libxl__device_remove(libxl__gc *gc, char *be_path);
+_hidden int libxl__device_destroy(libxl__gc *gc, char *be_path);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
tools/libxl/libxl_pci.c +1 -2
diff -r b8b0e5d41f32 -r 0584450afaf8 tools/libxl/libxl_pci.c
@@ -411,8 +411,7 @@ retry_transaction2:
 
     if (num == 1) {
         char *fe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/frontend", be_path));
-        libxl__device_destroy(gc, be_path, 1);
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
+        libxl__device_destroy(gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, fe_path);
         return 0;
     }
#20 lore raw

PATCH v2 1/1 libxl: split forced and non-forced uses of libxl__device_del

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 7e4dde7f0acccd73ef1296f4d8ce58d5ac0b2571 # Parent 0584450afaf8c1236f0c0b870fc16629c1233662
libxl: split forced and non-forced uses of libxl__device_del

Most forced users can now simply call libxl__device_destroy directly.

libxl__devices_destroy is something of a special case, it is really just iterating over an opaque set of xenstore directories and removing them. Until this can be refactored just do the force-remove case manually, doing otherwise led to too much entanglement with the other callers of libxl__device_destroy which do know about specific device types.

For the time being do the same in libxl__device_pci_remove_xenstore.

tools/libxl/libxl.c +9 -2
diff -r 0584450afaf8 -r 7e4dde7f0acc tools/libxl/libxl.c
@@ -1074,7 +1074,10 @@ int libxl_device_disk_del(libxl_ctx *ctx
     device.domid            = domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
-    rc = libxl__device_del(&gc, &device, wait);
+    if (wait)
+        rc = libxl__device_del(&gc, &device);
+    else
+        rc = libxl__device_destroy(&gc, &device);
 out_free:
     libxl__free_all(&gc);
     return rc;
@@ -1286,7 +1289,11 @@ int libxl_device_nic_del(libxl_ctx *ctx,
     device.domid            = domid;
     device.kind             = DEVICE_VIF;
 
-    rc = libxl__device_del(&gc, &device, wait);
+    if (wait)
+        rc = libxl__device_del(&gc, &device);
+    else
+        rc = libxl__device_destroy(&gc, &device);
+
     libxl__free_all(&gc);
     return rc;
 }
tools/libxl/libxl_device.c +28 -20

88 lines not on this page · raw message

tools/libxl/libxl_internal.h +2 -2
diff -r 0584450afaf8 -r 7e4dde7f0acc tools/libxl/libxl_internal.h
@@ -252,9 +252,9 @@ _hidden int libxl__device_generic_add(li
                              char **bents, char **fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
-_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev, int wait);
+_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_remove(libxl__gc *gc, char *be_path);
-_hidden int libxl__device_destroy(libxl__gc *gc, char *be_path);
+_hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
tools/libxl/libxl_pci.c +1 -1
diff -r 0584450afaf8 -r 7e4dde7f0acc tools/libxl/libxl_pci.c
@@ -411,7 +411,7 @@ retry_transaction2:
 
     if (num == 1) {
         char *fe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/frontend", be_path));
-        libxl__device_destroy(gc, be_path);
+        xs_rm(ctx->xsh, XBT_NULL, be_path);
         xs_rm(ctx->xsh, XBT_NULL, fe_path);
         return 0;
     }
#21 lore raw

PATCH v2 1/1 libxl: use IDL to define device front- and back-end kinds

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 98711982a532f09a7722fdd5af4e704562561b25 # Parent 7e4dde7f0acccd73ef1296f4d8ce58d5ac0b2571
libxl: use IDL to define device front- and back-end kinds

I'd like to use the from_string functionality...

tools/libxl/libxl.c +17 -17
diff -r 7e4dde7f0acc -r 98711982a532 tools/libxl/libxl.c
@@ -962,7 +962,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.backend_domid = disk->backend_domid;
     device.devid = devid;
     device.domid = domid;
-    device.kind = DEVICE_VBD;
+    device.kind = LIBXL__DEVICE_KIND_VBD;
 
     switch (disk->backend) {
         case LIBXL_DISK_BACKEND_PHY:
@@ -975,7 +975,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, dev);
 
-            device.backend_kind = DEVICE_VBD;
+            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
             break;
         case LIBXL_DISK_BACKEND_TAP:
             dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
@@ -994,7 +994,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
                           libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
-            device.backend_kind = DEVICE_QDISK;
+            device.backend_kind = LIBXL__DEVICE_KIND_QDISK;
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
@@ -1057,13 +1057,13 @@ int libxl_device_disk_del(libxl_ctx *ctx
 
     switch (disk->backend) {
         case LIBXL_DISK_BACKEND_PHY:
-            device.backend_kind = DEVICE_VBD;
+            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
             break;
         case LIBXL_DISK_BACKEND_TAP:
-            device.backend_kind = DEVICE_VBD;
+            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
             break;
         case LIBXL_DISK_BACKEND_QDISK:
-            device.backend_kind = DEVICE_QDISK;
+            device.backend_kind = LIBXL__DEVICE_KIND_QDISK;
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
@@ -1073,7 +1073,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
     }
     device.domid            = domid;
     device.devid            = devid;
-    device.kind             = DEVICE_VBD;
+    device.kind             = LIBXL__DEVICE_KIND_VBD;
     if (wait)
         rc = libxl__device_del(&gc, &device);
     else
@@ -1221,10 +1221,10 @@ int libxl_device_nic_add(libxl_ctx *ctx,
 
     device.backend_devid = nic->devid;
     device.backend_domid = nic->backend_domid;
-    device.backend_kind = DEVICE_VIF;
+    device.backend_kind = LIBXL__DEVICE_KIND_VIF;
     device.devid = nic->devid;
     device.domid = domid;
-    device.kind = DEVICE_VIF;
+    device.kind = LIBXL__DEVICE_KIND_VIF;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(&gc, "%d", domid));
@@ -1284,10 +1284,10 @@ int libxl_device_nic_del(libxl_ctx *ctx,
 
     device.backend_devid    = nic->devid;
     device.backend_domid    = nic->backend_domid;
-    device.backend_kind     = DEVICE_VIF;
+    device.backend_kind     = LIBXL__DEVICE_KIND_VIF;
     device.devid            = nic->devid;
     device.domid            = domid;
-    device.kind             = DEVICE_VIF;
+    device.kind             = LIBXL__DEVICE_KIND_VIF;
 
     if (wait)
         rc = libxl__device_del(&gc, &device);
@@ -1486,10 +1486,10 @@ int libxl__device_console_add(libxl__gc 
 
     device.backend_devid = console->devid;
     device.backend_domid = console->backend_domid;
-    device.backend_kind = DEVICE_CONSOLE;
+    device.backend_kind = LIBXL__DEVICE_KIND_CONSOLE;
     device.devid = console->devid;
     device.domid = domid;
-    device.kind = DEVICE_CONSOLE;
+    device.kind = LIBXL__DEVICE_KIND_CONSOLE;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(gc, "%d", domid));
@@ -1577,10 +1577,10 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
 
     device.backend_devid = vkb->devid;
     device.backend_domid = vkb->backend_domid;
-    device.backend_kind = DEVICE_VKBD;
+    device.backend_kind = LIBXL__DEVICE_KIND_VKBD;
     device.devid = vkb->devid;
     device.domid = domid;
-    device.kind = DEVICE_VKBD;
+    device.kind = LIBXL__DEVICE_KIND_VKBD;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(&gc, "%d", domid));
@@ -1864,10 +1864,10 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
 
     device.backend_devid = vfb->devid;
     device.backend_domid = vfb->backend_domid;
-    device.backend_kind = DEVICE_VFB;
+    device.backend_kind = LIBXL__DEVICE_KIND_VFB;
     device.devid = vfb->devid;
     device.domid = domid;
-    device.kind = DEVICE_VFB;
+    device.kind = LIBXL__DEVICE_KIND_VFB;
 
     flexarray_append_pair(back, "frontend-id", libxl__sprintf(&gc, "%d", domid));
     flexarray_append_pair(back, "online", "1");
tools/libxl/libxl_device.c +6 -23
diff -r 7e4dde7f0acc -r 98711982a532 tools/libxl/libxl_device.c
@@ -24,30 +24,20 @@
 #include <unistd.h>
 #include <fcntl.h>
 
-
 #include "libxl.h"
 #include "libxl_internal.h"
 
-static const char *string_of_kinds[] = {
-    [DEVICE_VIF] = "vif",
-    [DEVICE_VBD] = "vbd",
-    [DEVICE_QDISK] = "qdisk",
-    [DEVICE_PCI] = "pci",
-    [DEVICE_VFB] = "vfb",
-    [DEVICE_VKBD] = "vkbd",
-    [DEVICE_CONSOLE] = "console",
-};
-
 char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device)
 {
     char *dom_path = libxl__xs_get_dompath(gc, device->domid);
 
     /* Console 0 is a special case */
-    if (device->kind == DEVICE_CONSOLE && device->devid == 0)
+    if (device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0)
         return libxl__sprintf(gc, "%s/console", dom_path);
 
     return libxl__sprintf(gc, "%s/device/%s/%d", dom_path,
-                          string_of_kinds[device->kind], device->devid);
+                          libxl__device_kind_to_string(device->kind),
+                          device->devid);
 }
 
 char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device)
@@ -55,7 +45,7 @@ char *libxl__device_backend_path(libxl__
     char *dom_path = libxl__xs_get_dompath(gc, device->backend_domid);
 
     return libxl__sprintf(gc, "%s/backend/%s/%u/%d", dom_path,
-                          string_of_kinds[device->backend_kind],
+                          libxl__device_kind_to_string(device->backend_kind),
                           device->domid, device->devid);
 }
 
@@ -67,12 +57,6 @@ int libxl__device_generic_add(libxl__gc 
     xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
     struct xs_permissions backend_perms[2];
-    int rc;
-
-    if (!is_valid_device_kind(device->backend_kind) || !is_valid_device_kind(device->kind)) {
-        rc = ERROR_INVAL;
-        goto out;
-    }
 
     frontend_path = libxl__device_frontend_path(gc, device);
     backend_path = libxl__device_backend_path(gc, device);
@@ -113,9 +97,8 @@ retry_transaction:
         else
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs transaction failed");
     }
-    rc = 0;
-out:
-    return rc;
+
+    return 0;
 }
 
 typedef struct {
tools/libxl/libxl_internal.h +2 -14
diff -r 7e4dde7f0acc -r 98711982a532 tools/libxl/libxl_internal.h
@@ -97,25 +97,13 @@ struct libxl__ctx {
     libxl_version_info version_info;
 };
 
-typedef enum {
-    DEVICE_VIF = 1,
-    DEVICE_VBD,
-    DEVICE_QDISK,
-    DEVICE_PCI,
-    DEVICE_VFB,
-    DEVICE_VKBD,
-    DEVICE_CONSOLE,
-} libxl__device_kinds;
-
-#define is_valid_device_kind(kind) (((kind) >= DEVICE_VIF) && ((kind) <= DEVICE_CONSOLE))
-
 typedef struct {
     uint32_t backend_devid;
     uint32_t backend_domid;
     uint32_t devid;
     uint32_t domid;
-    libxl__device_kinds backend_kind;
-    libxl__device_kinds kind;
+    libxl__device_kind backend_kind;
+    libxl__device_kind kind;
 } libxl__device;
 
 #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
tools/libxl/libxl_pci.c +2 -2
diff -r 7e4dde7f0acc -r 98711982a532 tools/libxl/libxl_pci.c
@@ -244,10 +244,10 @@ int libxl__create_pci_backend(libxl__gc 
     /* add pci device */
     device.backend_devid = 0;
     device.backend_domid = 0;
-    device.backend_kind = DEVICE_PCI;
+    device.backend_kind = LIBXL__DEVICE_KIND_PCI;
     device.devid = 0;
     device.domid = domid;
-    device.kind = DEVICE_PCI;
+    device.kind = LIBXL__DEVICE_KIND_PCI;
 
     flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d", domid));
     flexarray_append_pair(back, "online", "1");
tools/libxl/libxl_types_internal.idl +11 -1
diff -r 7e4dde7f0acc -r 98711982a532 tools/libxl/libxl_types_internal.idl
@@ -1,9 +1,19 @@
 namespace("libxl__")
 
-libxl__qmp_message_type  = Enumeration("qmp_message_type", [
+libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
     (2, "return"),
     (3, "error"),
     (4, "event"),
     (5, "invalid"),
     ])
+
+libxl__device_kind = Enumeration("device_kind", [
+    (1, "VIF"),
+    (2, "VBD"),
+    (3, "QDISK"),
+    (4, "PCI"),
+    (5, "VFB"),
+    (6, "VKBD"),
+    (7, "CONSOLE"),
+    ])
#22 lore raw

PATCH v2 1/1 libxl: do not remove unidentified frontend paths in libxl__devices_destroy

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID c0636269da0c480e9f9a1a6c2d9126ad0e43a1ec
# Parent 98711982a532f09a7722fdd5af4e704562561b25
libxl: do not remove unidentified frontend paths in libxl__devices_destroy

Currently this appears to only include "/local/domain/<domid>/device/suspend". Ultimately the caller will nuke the whole guest directory anyway but not having this function remove things which don't look like devices seems less surprising.

tools/libxl/libxl_device.c +0 -2
diff -r 98711982a532 -r c0636269da0c tools/libxl/libxl_device.c
@@ -463,8 +463,6 @@ int libxl__devices_destroy(libxl__gc *gc
                     if (libxl__device_remove(gc, be_path) > 0)
                         n_watches++;
                 }
-            } else {
-                xs_rm(ctx->xsh, XBT_NULL, path);
             }
         }
     }
#23 lore raw

PATCH v2 1/1 libxl: use libxl__device in libxl_devices_destroy and libxl__device_pci_remove_xenstore

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID f60954a04df9acbddf68af4f2d46c3910f3b0afc
# Parent c0636269da0c480e9f9a1a6c2d9126ad0e43a1ec
libxl: use libxl__device in libxl_devices_destroy and libxl__device_pci_remove_xenstore

Doing this allows us to use the common functions for removing devices.

tools/libxl/libxl_device.c +46 -20
diff -r c0636269da0c -r f60954a04df9 tools/libxl/libxl_device.c
@@ -49,6 +49,25 @@ char *libxl__device_backend_path(libxl__
                           device->domid, device->devid);
 }
 
+int libxl__parse_backend_path(libxl__gc *gc,
+                              const char *path,
+                              libxl__device *dev)
+{
+    /* /local/domain/<domid>/backend/<kind>/<domid>/<devid> */
+    char strkind[16]; /* Longest is actually "console" */
+    uint32_t domain;
+    int rc = sscanf(path, "/local/domain/%d/backend/%15[^/]/%d/%d",
+                    &dev->backend_domid,
+                    strkind,
+                    &domain,
+                    &dev->backend_devid);
+
+    if (rc != 4)
+        return ERROR_FAIL;
+
+    return libxl__device_kind_from_string(strkind, &dev->backend_kind);
+}
+
 int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents)
 {
@@ -348,10 +367,11 @@ int libxl__device_disk_dev_number(const 
     return -1;
 }
 
-int libxl__device_remove(libxl__gc *gc, char *be_path)
+int libxl__device_remove(libxl__gc *gc, libxl__device *dev)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     xs_transaction_t t;
+    char *be_path = libxl__device_backend_path(gc, dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
     char *state = libxl__xs_read(gc, XBT_NULL, state_path);
     int rc = 0;
@@ -429,10 +449,12 @@ static int wait_for_dev_destroy(libxl__g
 int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    char *path, *be_path, *fe_path;
+    char *path;
     unsigned int num1, num2;
     char **l1 = NULL, **l2 = NULL;
     int i, j, n_watches = 0;
+    libxl__device dev;
+    libxl__device_kind kind;
 
     path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
     l1 = libxl__xs_directory(gc, XBT_NULL, path, &num1);
@@ -445,22 +467,25 @@ int libxl__devices_destroy(libxl__gc *gc
         num1 = 0;
     }
     for (i = 0; i < num1; i++) {
-        if (!strcmp("vfs", l1[i]))
+        if (libxl__device_kind_from_string(l1[i], &kind))
             continue;
         path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, l1[i]);
         l2 = libxl__xs_directory(gc, XBT_NULL, path, &num2);
         if (!l2)
             continue;
         for (j = 0; j < num2; j++) {
-            fe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", domid, l1[i], l2[j]);
-            be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path));
-            if (be_path != NULL) {
+            path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s/backend",
+                                  domid, l1[i], l2[j]);
+            path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, path));
+            if (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
+                dev.domid = domid;
+                dev.kind = kind;
+                dev.devid = atoi(l2[j]);
+
                 if (force) {
-                    xs_rm(ctx->xsh, XBT_NULL, be_path);
-                    xs_rm(ctx->xsh, XBT_NULL, fe_path);
-                    libxl__device_destroy_tapdisk(gc, be_path);
+                    libxl__device_destroy(gc, &dev);
                 } else {
-                    if (libxl__device_remove(gc, be_path) > 0)
+                    if (libxl__device_remove(gc, &dev) > 0)
                         n_watches++;
                 }
             }
@@ -468,14 +493,18 @@ int libxl__devices_destroy(libxl__gc *gc
     }
 
     /* console 0 frontend directory is not under /local/domain/<domid>/device */
-    fe_path = libxl__sprintf(gc, "/local/domain/%d/console", domid);
-    be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend", fe_path));
-    if (be_path && strcmp(be_path, "")) {
+    path = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid);
+    path = libxl__xs_read(gc, XBT_NULL, path);
+    if (path && strcmp(path, "") &&
+        libxl__parse_backend_path(gc, path, &dev) == 0) {
+        dev.domid = domid;
+        dev.kind = LIBXL__DEVICE_KIND_CONSOLE;
+        dev.devid = 0;
+
         if (force) {
-            xs_rm(ctx->xsh, XBT_NULL, be_path);
-            xs_rm(ctx->xsh, XBT_NULL, fe_path);
+            libxl__device_destroy(gc, &dev);
         } else {
-            if (libxl__device_remove(gc, be_path) > 0)
+            if (libxl__device_remove(gc, &dev) > 0)
                 n_watches++;
         }
     }
@@ -505,12 +534,9 @@ int libxl__device_del(libxl__gc *gc, lib
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     struct timeval tv;
-    char *backend_path;
     int rc;
 
-    backend_path = libxl__device_backend_path(gc, dev);
-
-    rc = libxl__device_remove(gc, backend_path);
+    rc = libxl__device_remove(gc, dev);
     if (rc == -1) {
         rc = ERROR_FAIL;
         goto out;
tools/libxl/libxl_internal.h +3 -1
diff -r c0636269da0c -r f60954a04df9 tools/libxl/libxl_internal.h
@@ -240,8 +240,10 @@ _hidden int libxl__device_generic_add(li
                              char **bents, char **fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
+_hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
+                                      libxl__device *dev);
 _hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__device_remove(libxl__gc *gc, char *be_path);
+_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
tools/libxl/libxl_pci.c +9 -3
diff -r c0636269da0c -r f60954a04df9 tools/libxl/libxl_pci.c
@@ -410,9 +410,15 @@ retry_transaction2:
             goto retry_transaction2;
 
     if (num == 1) {
-        char *fe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/frontend", be_path));
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
-        xs_rm(ctx->xsh, XBT_NULL, fe_path);
+        libxl__device dev;
+        if (libxl__parse_backend_path(gc, be_path, &dev) != 0)
+            return ERROR_FAIL;
+
+        dev.domid = domid;
+        dev.kind = LIBXL__DEVICE_KIND_PCI;
+        dev.devid = 0;
+
+        libxl__device_destroy(gc, &dev);
         return 0;
     }
#24 lore raw

PATCH v2 1/1 libxl: merge libxl__device_del into libxl__device_remove

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 95b2f3977d439bba3a01db4a4403a4307f238377 # Parent f60954a04df9acbddf68af4f2d46c3910f3b0afc
libxl: merge libxl__device_del into libxl__device_remove

Note that the "wait" parameter added to libxl_device_remove is different to the wait paramter previously used by similar functions. In the past not-wait meant forced whereas now in means wait for a graceful shutdown, as opposed to setting off a graceful shutdown but not waiting.

tools/libxl/libxl.c +2 -2
diff -r f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl.c
@@ -1075,7 +1075,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
     device.devid            = devid;
     device.kind             = LIBXL__DEVICE_KIND_VBD;
     if (wait)
-        rc = libxl__device_del(&gc, &device);
+        rc = libxl__device_remove(&gc, &device, wait);
     else
         rc = libxl__device_destroy(&gc, &device);
 out_free:
@@ -1290,7 +1290,7 @@ int libxl_device_nic_del(libxl_ctx *ctx,
     device.kind             = LIBXL__DEVICE_KIND_VIF;
 
     if (wait)
-        rc = libxl__device_del(&gc, &device);
+        rc = libxl__device_remove(&gc, &device, wait);
     else
         rc = libxl__device_destroy(&gc, &device);
 
tools/libxl/libxl_device.c +63 -76

169 lines not on this page · raw message

tools/libxl/libxl_internal.h +1 -2
diff -r f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl_internal.h
@@ -242,8 +242,7 @@ _hidden char *libxl__device_backend_path
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
-_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
+_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
#27 lore raw

PATCH v2 1/1 libxl: use more descriptive variable names in libxl__devices_destroy

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 53e6814c01f5e12baacc4d04e36904403e88ffbe
# Parent 95b2f3977d439bba3a01db4a4403a4307f238377
libxl: use more descriptive variable names in libxl__devices_destroy.

It's not immediately clear that "l1" iterates over device types and "l2" iterates over individual devices. Name things in a way which makes this more obvious.

tools/libxl/libxl_device.c +14 -13
diff -r 95b2f3977d43 -r 53e6814c01f5 tools/libxl/libxl_device.c
@@ -460,37 +460,38 @@ int libxl__devices_destroy(libxl__gc *gc
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *path;
-    unsigned int num1, num2;
-    char **l1 = NULL, **l2 = NULL;
+    unsigned int num_kinds, num_devs;
+    char **kinds = NULL, **devs = NULL;
     int i, j, n_watches = 0;
     libxl__device dev;
     libxl__device_kind kind;
 
     path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
-    l1 = libxl__xs_directory(gc, XBT_NULL, path, &num1);
-    if (!l1) {
+    kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
+    if (!kinds) {
         if (errno != ENOENT) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get xenstore"
                              " device listing %s", path);
             goto out;
         }
-        num1 = 0;
+        num_kinds = 0;
     }
-    for (i = 0; i < num1; i++) {
-        if (libxl__device_kind_from_string(l1[i], &kind))
+    for (i = 0; i < num_kinds; i++) {
+        if (libxl__device_kind_from_string(kinds[i], &kind))
             continue;
-        path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, l1[i]);
-        l2 = libxl__xs_directory(gc, XBT_NULL, path, &num2);
-        if (!l2)
+
+        path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, kinds[i]);
+        devs = libxl__xs_directory(gc, XBT_NULL, path, &num_devs);
+        if (!devs)
             continue;
-        for (j = 0; j < num2; j++) {
+        for (j = 0; j < num_devs; j++) {
             path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s/backend",
-                                  domid, l1[i], l2[j]);
+                                  domid, kinds[i], devs[j]);
             path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, path));
             if (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
                 dev.domid = domid;
                 dev.kind = kind;
-                dev.devid = atoi(l2[j]);
+                dev.devid = atoi(devs[j]);
 
                 if (force) {
                     libxl__device_destroy(gc, &dev);
#28 lore raw

PATCH v2 1/1 libxl: convert disk handling to device API

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID 2e302d27c6cac6c7a650b050366ce200e8cab50e # Parent 53e6814c01f5e12baacc4d04e36904403e88ffbe
libxl: convert disk handling to device API

tools/libxl/libxl.c +80 -49
diff -r 53e6814c01f5 -r 2e302d27c6ca tools/libxl/libxl.c
@@ -916,13 +916,58 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
+int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk)
+{
+    memset(disk, 0x00, sizeof(libxl_device_disk));
+    return 0;
+}
+
+static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+                                   libxl_device_disk *disk,
+                                   libxl__device *device)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int devid;
+
+    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
+    if (devid==-1) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+               " virtual disk identifier %s", disk->vdev);
+        return ERROR_INVAL;
+    }
+
+    device->backend_domid = disk->backend_domid;
+    device->backend_devid = devid;
+
+    switch (disk->backend) {
+        case LIBXL_DISK_BACKEND_PHY:
+            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+            break;
+        case LIBXL_DISK_BACKEND_TAP:
+            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+            break;
+        case LIBXL_DISK_BACKEND_QDISK:
+            device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
+                       disk->backend);
+            return ERROR_INVAL;
+    }
+
+    device->domid = domid;
+    device->devid = devid;
+    device->kind  = LIBXL__DEVICE_KIND_VBD;
+
+    return 0;
+}
+
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     flexarray_t *front;
     flexarray_t *back;
     char *dev;
-    int devid;
     libxl__device device;
     int major, minor, rc;
 
@@ -950,20 +995,13 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
-    if (devid==-1) {
+    rc = libxl__device_from_disk(&gc, domid, disk, &device);
+    if (rc != 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
                " virtual disk identifier %s", disk->vdev);
-        rc = ERROR_INVAL;
         goto out_free;
     }
 
-    device.backend_devid = devid;
-    device.backend_domid = disk->backend_domid;
-    device.devid = devid;
-    device.domid = domid;
-    device.kind = LIBXL__DEVICE_KIND_VBD;
-
     switch (disk->backend) {
         case LIBXL_DISK_BACKEND_PHY:
             dev = disk->pdev_path;
@@ -975,7 +1013,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, dev);
 
-            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
+            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
             break;
         case LIBXL_DISK_BACKEND_TAP:
             dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
@@ -994,7 +1032,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
                           libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
-            device.backend_kind = LIBXL__DEVICE_KIND_QDISK;
+            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
@@ -1026,7 +1064,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(front, "state");
     flexarray_append(front, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(front, "virtual-device");
-    flexarray_append(front, libxl__sprintf(&gc, "%d", devid));
+    flexarray_append(front, libxl__sprintf(&gc, "%d", device.devid));
     flexarray_append(front, "device-type");
     flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
@@ -1044,45 +1082,37 @@ out:
     return rc;
 }
 
-int libxl_device_disk_del(libxl_ctx *ctx, uint32_t domid,
-                          libxl_device_disk *disk, int wait)
+int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
+                             libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     libxl__device device;
-    int devid, rc;
-
-    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
-    device.backend_domid    = disk->backend_domid;
-    device.backend_devid    = devid;
-
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            device.backend_kind = LIBXL__DEVICE_KIND_VBD;
-            break;
-        case LIBXL_DISK_BACKEND_QDISK:
-            device.backend_kind = LIBXL__DEVICE_KIND_QDISK;
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
-                       disk->backend);
-            rc = ERROR_INVAL;
-            goto out_free;
-    }
-    device.domid            = domid;
-    device.devid            = devid;
-    device.kind             = LIBXL__DEVICE_KIND_VBD;
-    if (wait)
-        rc = libxl__device_remove(&gc, &device, wait);
-    else
-        rc = libxl__device_destroy(&gc, &device);
-out_free:
+    int rc;
+
+    rc = libxl__device_from_disk(&gc, domid, disk, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_remove(&gc, &device, 1);
+out:
     libxl__free_all(&gc);
     return rc;
 }
 
+int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_disk *disk)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_disk(&gc, domid, disk, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_destroy(&gc, &device);
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
 char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
@@ -1626,7 +1656,7 @@ static void libxl__device_disk_from_xs_b
     unsigned int len;
     char *tmp;
 
-    memset(disk, 0, sizeof(*disk));
+    libxl_device_disk_init(ctx, disk);
 
     tmp = xs_read(ctx->xsh, XBT_NULL,
                   libxl__sprintf(gc, "%s/params", be_path), &len);
@@ -1670,7 +1700,8 @@ int libxl_devid_to_device_disk(libxl_ctx
     char *dompath, *path;
     int rc = ERROR_FAIL;
 
-    memset(disk, 0, sizeof (libxl_device_disk));
+    libxl_device_disk_init(ctx, disk);
+
     dompath = libxl__xs_get_dompath(&gc, domid);
     if (!dompath) {
         goto out;
@@ -1812,11 +1843,11 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
 
     ret = 0;
 
-    libxl_device_disk_del(ctx, domid, disks + i, 1);
+    libxl_device_disk_remove(ctx, domid, disks + i);
     libxl_device_disk_add(ctx, domid, disk);
     stubdomid = libxl_get_stubdom_id(ctx, domid);
     if (stubdomid) {
-        libxl_device_disk_del(ctx, stubdomid, disks + i, 1);
+        libxl_device_disk_remove(ctx, stubdomid, disks + i);
         libxl_device_disk_add(ctx, stubdomid, disk);
     }
 out:
tools/libxl/libxl.h +14 -2
diff -r 53e6814c01f5 -r 2e302d27c6ca tools/libxl/libxl.h
@@ -438,15 +438,27 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *
  *   This function does not interact with the guest and therefore
  *   cannot block on the guest.
  */
+
+/* Disks */
+int libxl_device_disk_init(libxl_ctx *ctx, libxl_device_disk *disk);
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
-int libxl_device_disk_del(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk, int wait);
+int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
+int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_disk *disk);
+
 libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num);
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_disk *disk, libxl_diskinfo *diskinfo);
+
+/*
+ * Insert a CD-ROM device. A device corresponding to disk must already
+ * be attached to the guest.
+ */
 int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 
 /*
- * Make a disk available in this domain. Returns path to a device.
+ * Make a disk available in this (the control) domain. Returns path to
+ * a device.
  */
 char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
tools/libxl/xl_cmdimpl.c +5 -3
diff -r 53e6814c01f5 -r 2e302d27c6ca tools/libxl/xl_cmdimpl.c
@@ -494,7 +494,7 @@ static void parse_disk_config_multistrin
 {
     int e;
 
-    memset(disk, 0, sizeof(*disk));
+    libxl_device_disk_init(ctx, disk);
 
     if (!*config) {
         *config = xlu_cfg_init(stderr, "command line");
@@ -1893,6 +1893,8 @@ static void cd_insert(const char *dom, c
     disk.backend_domid = 0;
 
     libxl_cdrom_insert(ctx, domid, &disk);
+
+    libxl_device_disk_dispose(&disk);
     free(buf);
 }
 
@@ -4182,8 +4184,8 @@ int main_blockdetach(int argc, char **ar
         fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
-    if (libxl_device_disk_del(ctx, domid, &disk, 1)) {
-        fprintf(stderr, "libxl_device_disk_del failed.\n");
+    if (libxl_device_disk_remove(ctx, domid, &disk)) {
+        fprintf(stderr, "libxl_device_disk_remove failed.\n");
     }
     return 0;
 }
tools/ocaml/libs/xl/genwrap.py +6 -3
diff -r 53e6814c01f5 -r 2e302d27c6ca tools/ocaml/libs/xl/genwrap.py
@@ -16,6 +16,11 @@ builtins = {
     "libxl_hwcap":          ("int32 array",            None,                                "Val_hwcap(&%(c)s)"),
     }
 
+DEVICE_FUNCTIONS = [ ("add",            ["t", "domid", "unit"]),
+                     ("remove",         ["t", "domid", "unit"]),
+                     ("destroy",        ["t", "domid", "unit"]),
+                   ]
+
 functions = { # ( name , [type1,type2,....] )
     "device_vfb":     [ ("add",            ["t", "domid", "unit"]),
                         ("clean_shutdown", ["domid", "unit"]),
@@ -27,9 +32,7 @@ functions = { # ( name , [type1,type2,..
                       ],
     "device_console": [ ("add",            ["t", "domid", "unit"]),
                       ],
-    "device_disk":    [ ("add",            ["t", "domid", "unit"]),
-                        ("del",            ["t", "domid", "unit"]),
-                      ],
+    "device_disk":    DEVICE_FUNCTIONS,
     "device_nic":     [ ("add",            ["t", "domid", "unit"]),
                         ("del",            ["t", "domid", "unit"]),
                       ],
tools/ocaml/libs/xl/xenlight_stubs.c +1 -1
diff -r 53e6814c01f5 -r 2e302d27c6ca tools/ocaml/libs/xl/xenlight_stubs.c
@@ -247,7 +247,7 @@ value stub_xl_device_disk_del(value info
 	device_disk_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_disk_del(ctx, Int_val(domid), &c_info, 0);
+	ret = libxl_device_disk_remove(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("disk_del", &lg);
 	FREE_CTX();
#29 lore raw

PATCH v2 1/1 libxl: convert NIC handling to device API

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID 35a5d67ba9c81d489208786fe9f6ee803c67c82c # Parent 2e302d27c6cac6c7a650b050366ce200e8cab50e
libxl: convert NIC handling to device API

tools/libxl/libxl.c +56 -38
diff -r 2e302d27c6ca -r 35a5d67ba9c8 tools/libxl/libxl.c
@@ -1187,32 +1187,46 @@ int libxl_device_disk_local_detach(libxl
 }
 
 /******************************************************************************/
-int libxl_device_nic_init(libxl_device_nic *nic_info, int devnum)
+int libxl_device_nic_init(libxl_ctx *ctx, libxl_device_nic *nic)
 {
     const uint8_t *r;
     libxl_uuid uuid;
 
     libxl_uuid_generate(&uuid);
     r = libxl_uuid_bytearray(&uuid);
-    memset(nic_info, '\0', sizeof(*nic_info));
-
-    nic_info->backend_domid = 0;
-    nic_info->devid = devnum;
-    nic_info->mtu = 1492;
-    nic_info->model = strdup("rtl8139");
-    nic_info->mac[0] = 0x00;
-    nic_info->mac[1] = 0x16;
-    nic_info->mac[2] = 0x3e;
-    nic_info->mac[3] = r[0] & 0x7f;
-    nic_info->mac[4] = r[1];
-    nic_info->mac[5] = r[2];
-    nic_info->ifname = NULL;
-    nic_info->bridge = strdup("xenbr0");
-    nic_info->ip = NULL;
-    if ( asprintf(&nic_info->script, "%s/vif-bridge",
+    memset(nic, '\0', sizeof(*nic));
+
+    nic->backend_domid = 0;
+    nic->devid = -1;
+    nic->mtu = 1492;
+    nic->model = strdup("rtl8139");
+    nic->mac[0] = 0x00;
+    nic->mac[1] = 0x16;
+    nic->mac[2] = 0x3e;
+    nic->mac[3] = r[0] & 0x7f;
+    nic->mac[4] = r[1];
+    nic->mac[5] = r[2];
+    nic->ifname = NULL;
+    nic->bridge = strdup("xenbr0");
+    nic->ip = NULL;
+    if ( asprintf(&nic->script, "%s/vif-bridge",
                libxl_xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
-    nic_info->nictype = LIBXL_NIC_TYPE_IOEMU;
+    nic->nictype = LIBXL_NIC_TYPE_IOEMU;
+    return 0;
+}
+
+static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_nic *nic,
+                                  libxl__device *device)
+{
+    device->backend_devid    = nic->devid;
+    device->backend_domid    = nic->backend_domid;
+    device->backend_kind     = LIBXL__DEVICE_KIND_VIF;
+    device->devid            = nic->devid;
+    device->domid            = domid;
+    device->kind             = LIBXL__DEVICE_KIND_VIF;
+
     return 0;
 }
 
@@ -1249,12 +1263,8 @@ int libxl_device_nic_add(libxl_ctx *ctx,
         }
     }
 
-    device.backend_devid = nic->devid;
-    device.backend_domid = nic->backend_domid;
-    device.backend_kind = LIBXL__DEVICE_KIND_VIF;
-    device.devid = nic->devid;
-    device.domid = domid;
-    device.kind = LIBXL__DEVICE_KIND_VIF;
+    rc = libxl__device_from_nic(&gc, domid, nic, &device);
+    if ( rc != 0 ) goto out_free;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(&gc, "%d", domid));
@@ -1305,29 +1315,37 @@ out:
     return rc;
 }
 
-int libxl_device_nic_del(libxl_ctx *ctx, uint32_t domid,
-                         libxl_device_nic *nic, int wait)
+int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_nic *nic)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     libxl__device device;
     int rc;
 
-    device.backend_devid    = nic->devid;
-    device.backend_domid    = nic->backend_domid;
-    device.backend_kind     = LIBXL__DEVICE_KIND_VIF;
-    device.devid            = nic->devid;
-    device.domid            = domid;
-    device.kind             = LIBXL__DEVICE_KIND_VIF;
-
-    if (wait)
-        rc = libxl__device_remove(&gc, &device, wait);
-    else
-        rc = libxl__device_destroy(&gc, &device);
-
+    rc = libxl__device_from_nic(&gc, domid, nic, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_remove(&gc, &device, 1);
+out:
     libxl__free_all(&gc);
     return rc;
 }
 
+int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_nic *nic)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_nic(&gc, domid, nic, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_destroy(&gc, &device);
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
 static void libxl__device_nic_from_xs_be(libxl__gc *gc,
                                          const char *be_path,
                                          libxl_device_nic *nic)
tools/libxl/libxl.h +5 -2
diff -r 2e302d27c6ca -r 35a5d67ba9c8 tools/libxl/libxl.h
@@ -463,9 +463,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
 char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
 
-int libxl_device_nic_init(libxl_device_nic *nic, int dev_num);
+/* Network Interfaces */
+int libxl_device_nic_init(libxl_ctx *ctx, libxl_device_nic *nic);
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
-int libxl_device_nic_del(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic, int wait);
+int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
+
 libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num);
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
tools/libxl/xl_cmdimpl.c +4 -3
diff -r 2e302d27c6ca -r 35a5d67ba9c8 tools/libxl/xl_cmdimpl.c
@@ -762,7 +762,8 @@ static void parse_config_data(const char
 
             d_config->vifs = (libxl_device_nic *) realloc(d_config->vifs, sizeof (libxl_device_nic) * (d_config->num_vifs+1));
             nic = d_config->vifs + d_config->num_vifs;
-            CHK_ERRNO( libxl_device_nic_init(nic, d_config->num_vifs) );
+            CHK_ERRNO( libxl_device_nic_init(ctx, nic) );
+            nic->devid = d_config->num_vifs;
 
             if (default_vifscript) {
                 free(nic->script);
@@ -3972,7 +3973,7 @@ int main_networkattach(int argc, char **
         fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
-    libxl_device_nic_init(&nic, -1);
+    libxl_device_nic_init(ctx, &nic);
     for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
         if (MATCH_OPTION("type", *argv, oparg)) {
             if (!strcmp("vif", oparg)) {
@@ -4089,7 +4090,7 @@ int main_networkdetach(int argc, char **
             return 1;
         }
     }
-    if (libxl_device_nic_del(ctx, domid, &nic, 1)) {
+    if (libxl_device_nic_remove(ctx, domid, &nic)) {
         fprintf(stderr, "libxl_device_nic_del failed.\n");
         return 1;
     }
tools/ocaml/libs/xl/genwrap.py +1 -3
diff -r 2e302d27c6ca -r 35a5d67ba9c8 tools/ocaml/libs/xl/genwrap.py
@@ -33,9 +33,7 @@ functions = { # ( name , [type1,type2,..
     "device_console": [ ("add",            ["t", "domid", "unit"]),
                       ],
     "device_disk":    DEVICE_FUNCTIONS,
-    "device_nic":     [ ("add",            ["t", "domid", "unit"]),
-                        ("del",            ["t", "domid", "unit"]),
-                      ],
+    "device_nic":     DEVICE_FUNCTIONS,
     "device_pci":     [ ("add",            ["t", "domid", "unit"]),
                         ("remove",         ["t", "domid", "unit"]),
                         ("shutdown",       ["domid", "unit"]),
tools/ocaml/libs/xl/xenlight_stubs.c +1 -1
diff -r 2e302d27c6ca -r 35a5d67ba9c8 tools/ocaml/libs/xl/xenlight_stubs.c
@@ -281,7 +281,7 @@ value stub_xl_device_nic_del(value info,
 	device_nic_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_nic_del(ctx, Int_val(domid), &c_info, 0);
+	ret = libxl_device_nic_remove(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("nic_del", &lg);
 	FREE_CTX();
#30 lore raw

PATCH v2 1/1 libxl: remove libxl_device_console_add

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID 17b83c16a9a6b3ca77be2d42f8d1275c9c28e570 # Parent 35a5d67ba9c81d489208786fe9f6ee803c67c82c
libxl: remove libxl_device_console_add.

It has no callers, the only code which adds consoles in internal to libxl and uses libxl__device_console_add directly.

Rather than worrying about what the public API should look like in this case simply remove it, adding new APIs is much easier than fixing broken ones...

tools/libxl/libxl.c +0 -12
diff -r 35a5d67ba9c8 -r 17b83c16a9a6 tools/libxl/libxl.c
@@ -1585,18 +1585,6 @@ out:
     return rc;
 }
 
-int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_console *console)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    int rc = ERROR_INVAL;
-
-    rc = libxl__device_console_add(&gc, domid, console, NULL);
-
-    libxl__free_all(&gc);
-    return rc;
-}
-
 /******************************************************************************/
 void libxl_device_vkb_init(libxl_device_vkb *vkb, int dev_num)
 {
tools/libxl/libxl.h +0 -2
diff -r 35a5d67ba9c8 -r 17b83c16a9a6 tools/libxl/libxl.h
@@ -473,8 +473,6 @@ libxl_device_nic *libxl_device_nic_list(
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
-int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, libxl_device_console *console);
-
 void libxl_device_vkb_init(libxl_device_vkb *vkb, int dev_num);
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 int libxl_device_vkb_clean_shutdown(libxl_ctx *ctx, uint32_t domid);
tools/ocaml/libs/xl/genwrap.py +0 -2
diff -r 35a5d67ba9c8 -r 17b83c16a9a6 tools/ocaml/libs/xl/genwrap.py
@@ -30,8 +30,6 @@ functions = { # ( name , [type1,type2,..
                         ("clean_shutdown", ["domid", "unit"]),
                         ("hard_shutdown",  ["domid", "unit"]),
                       ],
-    "device_console": [ ("add",            ["t", "domid", "unit"]),
-                      ],
     "device_disk":    DEVICE_FUNCTIONS,
     "device_nic":     DEVICE_FUNCTIONS,
     "device_pci":     [ ("add",            ["t", "domid", "unit"]),
tools/ocaml/libs/xl/xenlight_stubs.c +0 -17
diff -r 35a5d67ba9c8 -r 17b83c16a9a6 tools/ocaml/libs/xl/xenlight_stubs.c
@@ -288,23 +288,6 @@ value stub_xl_device_nic_del(value info,
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_console_add(value info, value domid)
-{
-	CAMLparam2(info, domid);
-	libxl_device_console c_info;
-	int ret;
-	INIT_STRUCT();
-
-	device_console_val(&gc, &lg, &c_info, info);
-
-	INIT_CTX();
-	ret = libxl_device_console_add(ctx, Int_val(domid), &c_info);
-	if (ret != 0)
-		failwith_xl("console_add", &lg);
-	FREE_CTX();
-	CAMLreturn(Val_unit);
-}
-
 value stub_xl_device_vkb_add(value info, value domid)
 {
 	CAMLparam2(info, domid);
#31 lore raw

PATCH v2 1/1 libxl: convert VKB handling to device API

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID e161facb4c6a32411c5e3a73d5fdcf015f8a8591 # Parent 17b83c16a9a6b3ca77be2d42f8d1275c9c28e570
libxl: convert VKB handling to device API

tools/libxl/libxl.c +44 -22
diff -r 17b83c16a9a6 -r e161facb4c6a tools/libxl/libxl.c
@@ -1586,10 +1586,24 @@ out:
 }
 
 /******************************************************************************/
-void libxl_device_vkb_init(libxl_device_vkb *vkb, int dev_num)
+int libxl_device_vkb_init(libxl_ctx *ctx, libxl_device_vkb *vkb)
 {
     memset(vkb, 0x00, sizeof(libxl_device_vkb));
-    vkb->devid = dev_num;
+    return 0;
+}
+
+static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_vkb *vkb,
+                                  libxl__device *device)
+{
+    device->backend_devid = vkb->devid;
+    device->backend_domid = vkb->backend_domid;
+    device->backend_kind = LIBXL__DEVICE_KIND_VKBD;
+    device->devid = vkb->devid;
+    device->domid = domid;
+    device->kind = LIBXL__DEVICE_KIND_VKBD;
+
+    return 0;
 }
 
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb)
@@ -1611,12 +1625,8 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
         goto out_free;
     }
 
-    device.backend_devid = vkb->devid;
-    device.backend_domid = vkb->backend_domid;
-    device.backend_kind = LIBXL__DEVICE_KIND_VKBD;
-    device.devid = vkb->devid;
-    device.domid = domid;
-    device.kind = LIBXL__DEVICE_KIND_VKBD;
+    rc = libxl__device_from_vkb(&gc, domid, vkb, &device);
+    if (rc != 0) goto out_free;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(&gc, "%d", domid));
@@ -1644,14 +1654,36 @@ out:
     return rc;
 }
 
-int libxl_device_vkb_clean_shutdown(libxl_ctx *ctx, uint32_t domid)
+int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_vkb *vkb)
 {
-    return ERROR_NI;
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_vkb(&gc, domid, vkb, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_remove(&gc, &device, 1);
+out:
+    libxl__free_all(&gc);
+    return rc;
 }
 
-int libxl_device_vkb_hard_shutdown(libxl_ctx *ctx, uint32_t domid)
+int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_vkb *vkb)
 {
-    return ERROR_NI;
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_vkb(&gc, domid, vkb, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_destroy(&gc, &device);
+out:
+    libxl__free_all(&gc);
+    return rc;
 }
 
 static void libxl__device_disk_from_xs_be(libxl__gc *gc,
@@ -1939,16 +1971,6 @@ out:
     return rc;
 }
 
-int libxl_device_vfb_clean_shutdown(libxl_ctx *ctx, uint32_t domid)
-{
-    return ERROR_NI;
-}
-
-int libxl_device_vfb_hard_shutdown(libxl_ctx *ctx, uint32_t domid)
-{
-    return ERROR_NI;
-}
-
 /******************************************************************************/
 
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
tools/libxl/libxl.h +4 -3
diff -r 17b83c16a9a6 -r e161facb4c6a tools/libxl/libxl.h
@@ -473,10 +473,11 @@ libxl_device_nic *libxl_device_nic_list(
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
-void libxl_device_vkb_init(libxl_device_vkb *vkb, int dev_num);
+/* Keyboard */
+int libxl_device_vkb_init(libxl_ctx *ctx, libxl_device_vkb *vkb);
 int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
-int libxl_device_vkb_clean_shutdown(libxl_ctx *ctx, uint32_t domid);
-int libxl_device_vkb_hard_shutdown(libxl_ctx *ctx, uint32_t domid);
+int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
+int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 
 void libxl_device_vfb_init(libxl_device_vfb *vfb, int dev_num);
 int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
tools/libxl/xl_cmdimpl.c +2 -1
diff -r 17b83c16a9a6 -r e161facb4c6a tools/libxl/xl_cmdimpl.c
@@ -857,7 +857,8 @@ skip:
 
             d_config->vkbs = (libxl_device_vkb *) realloc(d_config->vkbs, sizeof(libxl_device_vkb) * (d_config->num_vkbs + 1));
             vkb = d_config->vkbs + d_config->num_vkbs;
-            libxl_device_vkb_init(vkb, d_config->num_vkbs);
+            libxl_device_vkb_init(ctx, vkb);
+            vkb->devid = d_config->num_vkbs;
 
             p = strtok(buf2, ",");
             if (!p)
tools/ocaml/libs/xl/genwrap.py +1 -4
diff -r 17b83c16a9a6 -r e161facb4c6a tools/ocaml/libs/xl/genwrap.py
@@ -26,10 +26,7 @@ functions = { # ( name , [type1,type2,..
                         ("clean_shutdown", ["domid", "unit"]),
                         ("hard_shutdown",  ["domid", "unit"]),
                       ],
-    "device_vkb":     [ ("add",            ["t", "domid", "unit"]),
-                        ("clean_shutdown", ["domid", "unit"]),
-                        ("hard_shutdown",  ["domid", "unit"]),
-                      ],
+    "device_vkb":     DEVICE_FUNCTIONS,
     "device_disk":    DEVICE_FUNCTIONS,
     "device_nic":     DEVICE_FUNCTIONS,
     "device_pci":     [ ("add",            ["t", "domid", "unit"]),
tools/ocaml/libs/xl/xenlight_stubs.c +10 -4
diff -r 17b83c16a9a6 -r e161facb4c6a tools/ocaml/libs/xl/xenlight_stubs.c
@@ -306,14 +306,17 @@ value stub_xl_device_vkb_add(value info,
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_vkb_clean_shutdown(value domid)
+value stub_xl_device_vkb_remove(value info, value domid)
 {
 	CAMLparam1(domid);
+	libxl_device_vkb c_info;
 	int ret;
 	INIT_STRUCT();
 
+	device_vkb_val(&gc, &lg, &c_info, info);
+
 	INIT_CTX();
-	ret = libxl_device_vkb_clean_shutdown(ctx, Int_val(domid));
+	ret = libxl_device_vkb_remove(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("vkb_clean_shutdown", &lg);
 	FREE_CTX();
@@ -321,14 +324,17 @@ value stub_xl_device_vkb_clean_shutdown(
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_vkb_hard_shutdown(value domid)
+value stub_xl_device_vkb_destroy(value info, value domid)
 {
 	CAMLparam1(domid);
+	libxl_device_vkb c_info;
 	int ret;
 	INIT_STRUCT();
 
+	device_vkb_val(&gc, &lg, &c_info, info);
+
 	INIT_CTX();
-	ret = libxl_device_vkb_hard_shutdown(ctx, Int_val(domid));
+	ret = libxl_device_vkb_destroy(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("vkb_hard_shutdown", &lg);
 	FREE_CTX();
#32 lore raw

PATCH v2 1/1 libxl: convert VFB handling to device API

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID 9abeaa8b4d3c1f21c4e1695211fa0ad2639eb586 # Parent e161facb4c6a32411c5e3a73d5fdcf015f8a8591
libxl: convert VFB handling to device API

tools/libxl/libxl.c +49 -8
diff -r e161facb4c6a -r 9abeaa8b4d3c tools/libxl/libxl.c
@@ -1896,10 +1896,9 @@ out:
 }
 
 /******************************************************************************/
-void libxl_device_vfb_init(libxl_device_vfb *vfb, int dev_num)
+int libxl_device_vfb_init(libxl_ctx *ctx, libxl_device_vfb *vfb)
 {
     memset(vfb, 0x00, sizeof(libxl_device_vfb));
-    vfb->devid = dev_num;
     vfb->display = NULL;
     vfb->xauthority = NULL;
     vfb->vnc = 1;
@@ -1910,6 +1909,20 @@ void libxl_device_vfb_init(libxl_device_
     vfb->keymap = NULL;
     vfb->sdl = 0;
     vfb->opengl = 0;
+    return 0;
+}
+
+static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_vfb *vfb,
+                                  libxl__device *device)
+{
+    device->backend_devid = vfb->devid;
+    device->backend_domid = vfb->backend_domid;
+    device->backend_kind = LIBXL__DEVICE_KIND_VFB;
+    device->devid = vfb->devid;
+    device->domid = domid;
+    device->kind = LIBXL__DEVICE_KIND_VFB;
+    return 0;
 }
 
 int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb)
@@ -1931,12 +1944,8 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
         goto out_free;
     }
 
-    device.backend_devid = vfb->devid;
-    device.backend_domid = vfb->backend_domid;
-    device.backend_kind = LIBXL__DEVICE_KIND_VFB;
-    device.devid = vfb->devid;
-    device.domid = domid;
-    device.kind = LIBXL__DEVICE_KIND_VFB;
+    rc = libxl__device_from_vfb(&gc, domid, vfb, &device);
+    if (rc != 0) goto out_free;
 
     flexarray_append_pair(back, "frontend-id", libxl__sprintf(&gc, "%d", domid));
     flexarray_append_pair(back, "online", "1");
@@ -1971,6 +1980,38 @@ out:
     return rc;
 }
 
+int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_vfb *vfb)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_vfb(&gc, domid, vfb, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_remove(&gc, &device, 1);
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
+int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_vfb *vfb)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl__device device;
+    int rc;
+
+    rc = libxl__device_from_vfb(&gc, domid, vfb, &device);
+    if (rc != 0) goto out;
+
+    rc = libxl__device_destroy(&gc, &device);
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
 /******************************************************************************/
 
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
tools/libxl/libxl.h +4 -3
diff -r e161facb4c6a -r 9abeaa8b4d3c tools/libxl/libxl.h
@@ -479,10 +479,11 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
 int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
 
-void libxl_device_vfb_init(libxl_device_vfb *vfb, int dev_num);
+/* Framebuffer */
+int libxl_device_vfb_init(libxl_ctx *ctx, libxl_device_vfb *vfb);
 int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
-int libxl_device_vfb_clean_shutdown(libxl_ctx *ctx, uint32_t domid);
-int libxl_device_vfb_hard_shutdown(libxl_ctx *ctx, uint32_t domid);
+int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
+int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 
 int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev, int force);
tools/libxl/xl_cmdimpl.c +2 -1
diff -r e161facb4c6a -r 9abeaa8b4d3c tools/libxl/xl_cmdimpl.c
@@ -853,7 +853,8 @@ skip:
 
             d_config->vfbs = (libxl_device_vfb *) realloc(d_config->vfbs, sizeof(libxl_device_vfb) * (d_config->num_vfbs + 1));
             vfb = d_config->vfbs + d_config->num_vfbs;
-            libxl_device_vfb_init(vfb, d_config->num_vfbs);
+            libxl_device_vfb_init(ctx, vfb);
+            vfb->devid = d_config->num_vfbs;
 
             d_config->vkbs = (libxl_device_vkb *) realloc(d_config->vkbs, sizeof(libxl_device_vkb) * (d_config->num_vkbs + 1));
             vkb = d_config->vkbs + d_config->num_vkbs;
tools/ocaml/libs/xl/genwrap.py +1 -4
diff -r e161facb4c6a -r 9abeaa8b4d3c tools/ocaml/libs/xl/genwrap.py
@@ -22,10 +22,7 @@ DEVICE_FUNCTIONS = [ ("add",            
                    ]
 
 functions = { # ( name , [type1,type2,....] )
-    "device_vfb":     [ ("add",            ["t", "domid", "unit"]),
-                        ("clean_shutdown", ["domid", "unit"]),
-                        ("hard_shutdown",  ["domid", "unit"]),
-                      ],
+    "device_vfb":     DEVICE_FUNCTIONS,
     "device_vkb":     DEVICE_FUNCTIONS,
     "device_disk":    DEVICE_FUNCTIONS,
     "device_nic":     DEVICE_FUNCTIONS,
tools/ocaml/libs/xl/xenlight_stubs.c +10 -4
diff -r e161facb4c6a -r 9abeaa8b4d3c tools/ocaml/libs/xl/xenlight_stubs.c
@@ -360,14 +360,17 @@ value stub_xl_device_vfb_add(value info,
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_vfb_clean_shutdown(value domid)
+value stub_xl_device_vfb_remove(value info, value domid)
 {
 	CAMLparam1(domid);
+	libxl_device_vfb c_info;
 	int ret;
 	INIT_STRUCT();
 
+	device_vfb_val(&gc, &lg, &c_info, info);
+
 	INIT_CTX();
-	ret = libxl_device_vfb_clean_shutdown(ctx, Int_val(domid));
+	ret = libxl_device_vfb_remove(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("vfb_clean_shutdown", &lg);
 	FREE_CTX();
@@ -375,14 +378,17 @@ value stub_xl_device_vfb_clean_shutdown(
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_vfb_hard_shutdown(value domid)
+value stub_xl_device_vfb_destroy(value info, value domid)
 {
 	CAMLparam1(domid);
+	libxl_device_vfb c_info;
 	int ret;
 	INIT_STRUCT();
 
+	device_vfb_val(&gc, &lg, &c_info, info);
+
 	INIT_CTX();
-	ret = libxl_device_vfb_hard_shutdown(ctx, Int_val(domid));
+	ret = libxl_device_vfb_destroy(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("vfb_hard_shutdown", &lg);
 	FREE_CTX();
#33 lore raw

PATCH v2 1/1 libxl: reorder device functions to put functions for each device together

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499566 -3600
# Node ID 5dcc40da135e09a00fcf8371dc452b4334fb6397
# Parent 9abeaa8b4d3c1f21c4e1695211fa0ad2639eb586
libxl: reorder device functions to put functions for each device together.

tools/libxl/libxl.c +211 -209
diff -r 9abeaa8b4d3c -r 5dcc40da135e tools/libxl/libxl.c
@@ -1113,6 +1113,216 @@ out:
     libxl__free_all(&gc);
     return rc;
 }
+
+static void libxl__device_disk_from_xs_be(libxl__gc *gc,
+                                          const char *be_path,
+                                          libxl_device_disk *disk)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    unsigned int len;
+    char *tmp;
+
+    libxl_device_disk_init(ctx, disk);
+
+    tmp = xs_read(ctx->xsh, XBT_NULL,
+                  libxl__sprintf(gc, "%s/params", be_path), &len);
+    if (tmp && strchr(tmp, ':')) {
+        disk->pdev_path = strdup(strchr(tmp, ':') + 1);
+        free(tmp);
+    } else {
+        disk->pdev_path = tmp;
+    }
+    libxl_string_to_backend(ctx,
+                        libxl__xs_read(gc, XBT_NULL,
+                                       libxl__sprintf(gc, "%s/type", be_path)),
+                        &(disk->backend));
+    disk->vdev = xs_read(ctx->xsh, XBT_NULL,
+                         libxl__sprintf(gc, "%s/dev", be_path), &len);
+    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf
+                         (gc, "%s/removable", be_path));
+
+    if (tmp)
+        disk->removable = atoi(tmp);
+    else
+        disk->removable = 0;
+
+    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/mode", be_path));
+    if (!strcmp(tmp, "w"))
+        disk->readwrite = 1;
+    else
+        disk->readwrite = 0;
+
+    tmp = libxl__xs_read(gc, XBT_NULL,
+                         libxl__sprintf(gc, "%s/device-type", be_path));
+    disk->is_cdrom = !strcmp(tmp, "cdrom");
+
+    disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
+}
+
+int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
+                               int devid, libxl_device_disk *disk)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *dompath, *path;
+    int rc = ERROR_FAIL;
+
+    libxl_device_disk_init(ctx, disk);
+
+    dompath = libxl__xs_get_dompath(&gc, domid);
+    if (!dompath) {
+        goto out;
+    }
+    path = libxl__xs_read(&gc, XBT_NULL,
+                          libxl__sprintf(&gc, "%s/device/vbd/%d/backend",
+                                         dompath, devid));
+    if (!path)
+        goto out;
+
+    libxl__device_disk_from_xs_be(&gc, path, disk);
+
+    rc = 0;
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
+
+static int libxl__append_disk_list_of_type(libxl__gc *gc,
+                                           uint32_t domid,
+                                           const char *type,
+                                           libxl_device_disk **disks,
+                                           int *ndisks)
+{
+    char *be_path = NULL;
+    char **dir = NULL;
+    unsigned int n = 0;
+    libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
+
+    be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
+                             libxl__xs_get_dompath(gc, 0), type, domid);
+    dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
+    if (dir) {
+        libxl_device_disk *tmp;
+        tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
+        if (tmp == NULL)
+            return ERROR_NOMEM;
+        *disks = tmp;
+        pdisk = *disks + *ndisks;
+        *ndisks += n;
+        pdisk_end = *disks + *ndisks;
+        for (; pdisk < pdisk_end; pdisk++, dir++) {
+            const char *p;
+            p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
+            libxl__device_disk_from_xs_be(gc, p, pdisk);
+            pdisk->backend_domid = 0;
+        }
+    }
+    return 0;
+}
+
+libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    libxl_device_disk *disks = NULL;
+    int rc;
+
+    *num = 0;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "vbd", &disks, num);
+    if (rc) goto out_err;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "tap", &disks, num);
+    if (rc) goto out_err;
+
+    rc = libxl__append_disk_list_of_type(&gc, domid, "qdisk", &disks, num);
+    if (rc) goto out_err;
+
+    libxl__free_all(&gc);
+    return disks;
+
+out_err:
+    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list disks");
+    while (disks && *num) {
+        (*num)--;
+        libxl_device_disk_dispose(&disks[*num]);
+    }
+    free(disks);
+    return NULL;
+}
+
+int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_disk *disk, libxl_diskinfo *diskinfo)
+{
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
+    char *dompath, *diskpath;
+    char *val;
+
+    dompath = libxl__xs_get_dompath(&gc, domid);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
+
+    /* tap devices entries in xenstore are written as vbd devices. */
+    diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);
+    diskinfo->backend = xs_read(ctx->xsh, XBT_NULL,
+                                libxl__sprintf(&gc, "%s/backend", diskpath), NULL);
+    if (!diskinfo->backend) {
+        libxl__free_all(&gc);
+        return ERROR_FAIL;
+    }
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend-id", diskpath));
+    diskinfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/state", diskpath));
+    diskinfo->state = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/event-channel", diskpath));
+    diskinfo->evtch = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/ring-ref", diskpath));
+    diskinfo->rref = val ? strtoul(val, NULL, 10) : -1;
+    diskinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                 libxl__sprintf(&gc, "%s/frontend", diskinfo->backend), NULL);
+    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/frontend-id", diskinfo->backend));
+    diskinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+
+    libxl__free_all(&gc);
+    return 0;
+}
+
+int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+{
+    int num, i;
+    uint32_t stubdomid;
+    libxl_device_disk *disks;
+    int ret = ERROR_FAIL;
+
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = LIBXL_DISK_FORMAT_EMPTY;
+    }
+    disks = libxl_device_disk_list(ctx, domid, &num);
+    for (i = 0; i < num; i++) {
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
+            /* found */
+            break;
+    }
+    if (i == num) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual device not found");
+        goto out;
+    }
+
+    ret = 0;
+
+    libxl_device_disk_remove(ctx, domid, disks + i);
+    libxl_device_disk_add(ctx, domid, disk);
+    stubdomid = libxl_get_stubdom_id(ctx, domid);
+    if (stubdomid) {
+        libxl_device_disk_remove(ctx, stubdomid, disks + i);
+        libxl_device_disk_add(ctx, stubdomid, disk);
+    }
+out:
+    for (i = 0; i < num; i++)
+        libxl_device_disk_dispose(&disks[i]);
+    free(disks);
+    return ret;
+}
+
 char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
@@ -1346,6 +1556,7 @@ out:
     libxl__free_all(&gc);
     return rc;
 }
+
 static void libxl__device_nic_from_xs_be(libxl__gc *gc,
                                          const char *be_path,
                                          libxl_device_nic *nic)
@@ -1686,215 +1897,6 @@ out:
     return rc;
 }
 
-static void libxl__device_disk_from_xs_be(libxl__gc *gc,
-                                          const char *be_path,
-                                          libxl_device_disk *disk)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    unsigned int len;
-    char *tmp;
-
-    libxl_device_disk_init(ctx, disk);
-
-    tmp = xs_read(ctx->xsh, XBT_NULL,
-                  libxl__sprintf(gc, "%s/params", be_path), &len);
-    if (tmp && strchr(tmp, ':')) {
-        disk->pdev_path = strdup(strchr(tmp, ':') + 1);
-        free(tmp);
-    } else {
-        disk->pdev_path = tmp;
-    }
-    libxl_string_to_backend(ctx,
-                        libxl__xs_read(gc, XBT_NULL,
-                                       libxl__sprintf(gc, "%s/type", be_path)),
-                        &(disk->backend));
-    disk->vdev = xs_read(ctx->xsh, XBT_NULL,
-                         libxl__sprintf(gc, "%s/dev", be_path), &len);
-    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf
-                         (gc, "%s/removable", be_path));
-
-    if (tmp)
-        disk->removable = atoi(tmp);
-    else
-        disk->removable = 0;
-
-    tmp = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/mode", be_path));
-    if (!strcmp(tmp, "w"))
-        disk->readwrite = 1;
-    else
-        disk->readwrite = 0;
-
-    tmp = libxl__xs_read(gc, XBT_NULL,
-                         libxl__sprintf(gc, "%s/device-type", be_path));
-    disk->is_cdrom = !strcmp(tmp, "cdrom");
-
-    disk->format = LIBXL_DISK_FORMAT_UNKNOWN;
-}
-
-int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
-                               int devid, libxl_device_disk *disk)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dompath, *path;
-    int rc = ERROR_FAIL;
-
-    libxl_device_disk_init(ctx, disk);
-
-    dompath = libxl__xs_get_dompath(&gc, domid);
-    if (!dompath) {
-        goto out;
-    }
-    path = libxl__xs_read(&gc, XBT_NULL,
-                          libxl__sprintf(&gc, "%s/device/vbd/%d/backend",
-                                         dompath, devid));
-    if (!path)
-        goto out;
-
-    libxl__device_disk_from_xs_be(&gc, path, disk);
-
-    rc = 0;
-out:
-    libxl__free_all(&gc);
-    return rc;
-}
-
-
-static int libxl__append_disk_list_of_type(libxl__gc *gc,
-                                           uint32_t domid,
-                                           const char *type,
-                                           libxl_device_disk **disks,
-                                           int *ndisks)
-{
-    char *be_path = NULL;
-    char **dir = NULL;
-    unsigned int n = 0;
-    libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
-
-    be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
-                             libxl__xs_get_dompath(gc, 0), type, domid);
-    dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
-    if (dir) {
-        libxl_device_disk *tmp;
-        tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
-        if (tmp == NULL)
-            return ERROR_NOMEM;
-        *disks = tmp;
-        pdisk = *disks + *ndisks;
-        *ndisks += n;
-        pdisk_end = *disks + *ndisks;
-        for (; pdisk < pdisk_end; pdisk++, dir++) {
-            const char *p;
-            p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
-            libxl__device_disk_from_xs_be(gc, p, pdisk);
-            pdisk->backend_domid = 0;
-        }
-    }
-    return 0;
-}
-
-libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    libxl_device_disk *disks = NULL;
-    int rc;
-
-    *num = 0;
-
-    rc = libxl__append_disk_list_of_type(&gc, domid, "vbd", &disks, num);
-    if (rc) goto out_err;
-
-    rc = libxl__append_disk_list_of_type(&gc, domid, "tap", &disks, num);
-    if (rc) goto out_err;
-
-    rc = libxl__append_disk_list_of_type(&gc, domid, "qdisk", &disks, num);
-    if (rc) goto out_err;
-
-    libxl__free_all(&gc);
-    return disks;
-
-out_err:
-    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list disks");
-    while (disks && *num) {
-        (*num)--;
-        libxl_device_disk_dispose(&disks[*num]);
-    }
-    free(disks);
-    return NULL;
-}
-
-int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_disk *disk, libxl_diskinfo *diskinfo)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *dompath, *diskpath;
-    char *val;
-
-    dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
-
-    /* tap devices entries in xenstore are written as vbd devices. */
-    diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);
-    diskinfo->backend = xs_read(ctx->xsh, XBT_NULL,
-                                libxl__sprintf(&gc, "%s/backend", diskpath), NULL);
-    if (!diskinfo->backend) {
-        libxl__free_all(&gc);
-        return ERROR_FAIL;
-    }
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend-id", diskpath));
-    diskinfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/state", diskpath));
-    diskinfo->state = val ? strtoul(val, NULL, 10) : -1;
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/event-channel", diskpath));
-    diskinfo->evtch = val ? strtoul(val, NULL, 10) : -1;
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/ring-ref", diskpath));
-    diskinfo->rref = val ? strtoul(val, NULL, 10) : -1;
-    diskinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
-                                 libxl__sprintf(&gc, "%s/frontend", diskinfo->backend), NULL);
-    val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/frontend-id", diskinfo->backend));
-    diskinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
-
-    libxl__free_all(&gc);
-    return 0;
-}
-
-int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
-{
-    int num, i;
-    uint32_t stubdomid;
-    libxl_device_disk *disks;
-    int ret = ERROR_FAIL;
-
-    if (!disk->pdev_path) {
-        disk->pdev_path = strdup("");
-        disk->format = LIBXL_DISK_FORMAT_EMPTY;
-    }
-    disks = libxl_device_disk_list(ctx, domid, &num);
-    for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
-            /* found */
-            break;
-    }
-    if (i == num) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual device not found");
-        goto out;
-    }
-
-    ret = 0;
-
-    libxl_device_disk_remove(ctx, domid, disks + i);
-    libxl_device_disk_add(ctx, domid, disk);
-    stubdomid = libxl_get_stubdom_id(ctx, domid);
-    if (stubdomid) {
-        libxl_device_disk_remove(ctx, stubdomid, disks + i);
-        libxl_device_disk_add(ctx, stubdomid, disk);
-    }
-out:
-    for (i = 0; i < num; i++)
-        libxl_device_disk_dispose(&disks[i]);
-    free(disks);
-    return ret;
-}
-
 /******************************************************************************/
 int libxl_device_vfb_init(libxl_ctx *ctx, libxl_device_vfb *vfb)
 {
#34 lore raw

PATCH v2 1/1 libxl: convert PCI device handling to device API

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID 090e45b2fdcadd82d7373db52d84d1c0e5442a29 # Parent 5dcc40da135e09a00fcf8371dc452b4334fb6397
libxl: convert PCI device handling to device API

tools/libxl/libxl.c +1 -1
diff -r 5dcc40da135e -r 090e45b2fdca tools/libxl/libxl.c
@@ -754,7 +754,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
         goto out;
     }
 
-    if (libxl_device_pci_shutdown(ctx, domid) < 0)
+    if (libxl__device_pci_destroy_all(&gc, domid) < 0)
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "pci shutdown failed for domid %d", domid);
     rc = xc_domain_pause(ctx->xch, domid);
     if (rc < 0) {
tools/libxl/libxl.h +21 -5
diff -r 5dcc40da135e -r 090e45b2fdca tools/libxl/libxl.h
@@ -485,12 +485,28 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
 int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);
 
+/* PCI Passthrough */
+int libxl_device_pci_init(libxl_ctx *ctx, libxl_device_pci *pci);
 int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
-int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev, int force);
-int libxl_device_pci_shutdown(libxl_ctx *ctx, uint32_t domid);
-int libxl_device_pci_list_assigned(libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num);
-int libxl_device_pci_list_assignable(libxl_ctx *ctx, libxl_device_pci **list, int *num);
-int libxl_device_pci_parse_bdf(libxl_ctx *ctx, libxl_device_pci *pcidev, const char *str);
+int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
+int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
+libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
+
+/*
+ * Parse a PCI BDF into a PCI device structure.
+ */
+int libxl_device_pci_parse_bdf(libxl_ctx *ctx,
+                               libxl_device_pci *pcidev,
+                               const char *str);
+
+/*
+ * Similar to libxl_device_pci_list but returns all devices which
+ * could be assigned to a domain (i.e. are bound to the backend
+ * driver) but are not currently.
+ */
+libxl_device_pci *libxl_device_pci_list_assignable(libxl_ctx *ctx, int *num);
+
+/* CPUID handling */
 int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str);
 int libxl_cpuid_parse_config_xend(libxl_cpuid_policy_list *cpuid,
                                   const char* str);
tools/libxl/libxl_internal.h +1 -0
diff -r 5dcc40da135e -r 090e45b2fdca tools/libxl/libxl_internal.h
@@ -252,6 +252,7 @@ _hidden int libxl__wait_for_backend(libx
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
 _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
                                       libxl_device_pci *pcidev, int num);
+_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
 
 /* xl_exec */
 
tools/libxl/libxl_pci.c +115 -71

287 lines not on this page · raw message

tools/libxl/xl_cmdimpl.c +11 -4
diff -r 5dcc40da135e -r 090e45b2fdca tools/libxl/xl_cmdimpl.c
@@ -2018,11 +2018,14 @@ static void pcilist_assignable(void)
     libxl_device_pci *pcidevs;
     int num, i;
 
-    if ( libxl_device_pci_list_assignable(ctx, &pcidevs, &num) )
+    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
+
+    if ( pcidevs == NULL )
         return;
     for (i = 0; i < num; i++) {
         printf("%04x:%02x:%02x.%01x\n",
-                pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+               pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+        libxl_device_pci_dispose(&pcidevs[i]);
     }
     free(pcidevs);
 }
@@ -2045,7 +2048,8 @@ static void pcilist(const char *dom)
 
     find_domain(dom);
 
-    if (libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num))
+    pcidevs = libxl_device_pci_list(ctx, domid, &num);
+    if (pcidevs == NULL)
         return;
     printf("Vdev Device\n");
     for (i = 0; i < num; i++) {
@@ -2082,7 +2086,10 @@ static void pcidetach(const char *dom, c
         fprintf(stderr, "pci-detach: malformed BDF specification \"%s\"\n", bdf);
         exit(2);
     }
-    libxl_device_pci_remove(ctx, domid, &pcidev, force);
+    if (force)
+        libxl_device_pci_destroy(ctx, domid, &pcidev);
+    else
+        libxl_device_pci_remove(ctx, domid, &pcidev);
     libxl_device_pci_dispose(&pcidev);
 }
 
tools/ocaml/libs/xl/genwrap.py +1 -4
diff -r 5dcc40da135e -r 090e45b2fdca tools/ocaml/libs/xl/genwrap.py
@@ -26,10 +26,7 @@ functions = { # ( name , [type1,type2,..
     "device_vkb":     DEVICE_FUNCTIONS,
     "device_disk":    DEVICE_FUNCTIONS,
     "device_nic":     DEVICE_FUNCTIONS,
-    "device_pci":     [ ("add",            ["t", "domid", "unit"]),
-                        ("remove",         ["t", "domid", "unit"]),
-                        ("shutdown",       ["domid", "unit"]),
-                      ],
+    "device_pci":     DEVICE_FUNCTIONS,
     "physinfo":       [ ("get",            ["unit", "t"]),
                       ],
     "sched_credit":   [ ("domain_get",     ["domid", "t"]),
tools/ocaml/libs/xl/xenlight_stubs.c +1 -16
diff -r 5dcc40da135e -r 090e45b2fdca tools/ocaml/libs/xl/xenlight_stubs.c
@@ -424,7 +424,7 @@ value stub_xl_device_pci_remove(value in
 	device_pci_val(&gc, &lg, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_pci_remove(ctx, Int_val(domid), &c_info, 0);
+	ret = libxl_device_pci_remove(ctx, Int_val(domid), &c_info);
 	if (ret != 0)
 		failwith_xl("pci_remove", &lg);
 	FREE_CTX();
@@ -432,21 +432,6 @@ value stub_xl_device_pci_remove(value in
 	CAMLreturn(Val_unit);
 }
 
-value stub_xl_device_pci_shutdown(value domid)
-{
-	CAMLparam1(domid);
-	int ret;
-	INIT_STRUCT();
-
-	INIT_CTX();
-	ret = libxl_device_pci_shutdown(ctx, Int_val(domid));
-	if (ret != 0)
-		failwith_xl("pci_shutdown", &lg);
-	FREE_CTX();
-
-	CAMLreturn(Val_unit);
-}
-
 value stub_xl_button_press(value domid, value button)
 {
 	CAMLparam2(domid, button);
tools/python/xen/lowlevel/xl/xl.c +14 -5
diff -r 5dcc40da135e -r 090e45b2fdca tools/python/xen/lowlevel/xl/xl.c
@@ -521,9 +521,16 @@ static PyObject *pyxl_pci_del(XlObject *
         return NULL;
     }
     pci = (Py_device_pci *)obj;
-    if ( libxl_device_pci_remove(self->ctx, domid, &pci->obj, force) ) {
-        PyErr_SetString(xl_error_obj, "cannot remove pci device");
-        return NULL;
+    if ( force ) {
+        if ( libxl_device_pci_destroy(self->ctx, domid, &pci->obj) ) {
+            PyErr_SetString(xl_error_obj, "cannot remove pci device");
+            return NULL;
+        }
+    } else {
+        if ( libxl_device_pci_remove(self->ctx, domid, &pci->obj) ) {
+            PyErr_SetString(xl_error_obj, "cannot remove pci device");
+            return NULL;
+        }
     }
     Py_INCREF(Py_None);
     return Py_None;
@@ -558,7 +565,8 @@ static PyObject *pyxl_pci_list_assignabl
     PyObject *list;
     int nr_dev, i;
 
-    if ( libxl_device_pci_list_assignable(self->ctx, &dev, &nr_dev) ) {
+    dev = libxl_device_pci_list_assignable(self->ctx, &nr_dev);
+    if ( dev == NULL ) {
         PyErr_SetString(xl_error_obj, "Cannot list assignable devices");
         return NULL;
     }
@@ -594,7 +602,8 @@ static PyObject *pyxl_pci_list(XlObject 
     if ( !PyArg_ParseTuple(args, "i", &domid) )
         return NULL;
 
-    if ( libxl_device_pci_list_assigned(self->ctx, &dev, domid, &nr_dev) ) {
+    dev = libxl_device_pci_list(self->ctx, domid, &nr_dev);
+    if ( dev == NULL ) {
         PyErr_SetString(xl_error_obj, "Cannot list assignable devices");
         return NULL;
     }
#35 lore raw

PATCH v2 1/1 imported patch libxl-flags-on-ctx-alloc.patch

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID ed4bd6824e5e4f751bf1b77a70a514111aaa077e # Parent 090e45b2fdcadd82d7373db52d84d1c0e5442a29 imported patch libxl-flags-on-ctx-alloc.patch

tools/libxl/gentest.py +1 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/gentest.py
@@ -239,7 +239,7 @@ int main(int argc, char **argv)
     logger = xtl_createlogger_stdiostream(stderr, XTL_DETAIL, 0);
     if (!logger) exit(1);
 
-    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, (xentoollog_logger*)logger)) {
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
         fprintf(stderr, "cannot init xl context\\n");
         exit(1);
     }
tools/libxl/libxl.c +2 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/libxl.c
@@ -39,7 +39,8 @@
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
 
-int libxl_ctx_alloc(libxl_ctx **pctx, int version, xentoollog_logger * lg)
+int libxl_ctx_alloc(libxl_ctx **pctx, int version,
+                    unsigned flags, xentoollog_logger * lg)
 {
     libxl_ctx *ctx;
     struct stat stat_buf;
tools/libxl/libxl.h +3 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/libxl.h
@@ -245,7 +245,9 @@ typedef struct {
 } libxl_domain_config;
 
 /* context functions */
-int libxl_ctx_alloc(libxl_ctx **pctx, int version, xentoollog_logger *lg);
+int libxl_ctx_alloc(libxl_ctx **pctx, int version,
+                    unsigned flags /* none currently defined */,
+                    xentoollog_logger *lg);
 int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */);
 int libxl_ctx_postfork(libxl_ctx *ctx);
 
tools/libxl/xl.c +1 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/xl.c
@@ -116,7 +116,7 @@ int main(int argc, char **argv)
     logger = xtl_createlogger_stdiostream(stderr, minmsglevel,  0);
     if (!logger) exit(1);
 
-    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, (xentoollog_logger*)logger)) {
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
         fprintf(stderr, "cannot init xl context\n");
         exit(1);
     }
tools/ocaml/libs/xl/xenlight_stubs.c +1 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/ocaml/libs/xl/xenlight_stubs.c
@@ -59,7 +59,7 @@ static void log_destroy(struct xentoollo
 	lg.logger.destroy = log_destroy; \
 	lg.logger.progress = NULL; \
 	caml_enter_blocking_section(); \
-	ret = libxl_ctx_alloc(&ctx, LIBXL_VERSION, (struct xentoollog_logger *) &lg); \
+	ret = libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (struct xentoollog_logger *) &lg); \
 	if (ret != 0) \
 		failwith_xl("cannot init context", &lg);
 
tools/python/xen/lowlevel/xl/xl.c +1 -1
diff -r 090e45b2fdca -r ed4bd6824e5e tools/python/xen/lowlevel/xl/xl.c
@@ -687,7 +687,7 @@ PyXl_init(XlObject *self, PyObject *args
         return -1;
     }
 
-    if ( libxl_ctx_alloc(&self->ctx, LIBXL_VERSION,
+    if ( libxl_ctx_alloc(&self->ctx, LIBXL_VERSION, 0,
                 (xentoollog_logger*)self->logger) ) {
         PyErr_SetString(xl_error_obj, "cannot init xl context");
         return -1;
#36 lore raw

Re: [PATCH 27 of 27 v2] imported patch libxl-flags-on-ctx-alloc.patch

Quoted 6 lines · Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1318499566 -3600
# Node ID ed4bd6824e5e4f751bf1b77a70a514111aaa077e # Parent 090e45b2fdcadd82d7373db52d84d1c0e5442a29 imported patch libxl-flags-on-ctx-alloc.patch

Hmm. seems the comment for this one didn't get done right. Ignore it for now, it was more of an addendum anyway.

Quoted 76 lines

diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/gentest.py

tools/libxl/gentest.py +1 -1
@@ -239,7 +239,7 @@ int main(int argc, char **argv)
     logger = xtl_createlogger_stdiostream(stderr, XTL_DETAIL, 0);
     if (!logger) exit(1);
 
-    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, (xentoollog_logger*)logger)) {
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
         fprintf(stderr, "cannot init xl context\\n");
         exit(1);
     }

diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/libxl.c

tools/libxl/libxl.c +2 -1
@@ -39,7 +39,8 @@
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
 
-int libxl_ctx_alloc(libxl_ctx **pctx, int version, xentoollog_logger * lg)
+int libxl_ctx_alloc(libxl_ctx **pctx, int version,
+                    unsigned flags, xentoollog_logger * lg)
 {
     libxl_ctx *ctx;
     struct stat stat_buf;

diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/libxl.h

tools/libxl/libxl.h +3 -1
@@ -245,7 +245,9 @@ typedef struct {
 } libxl_domain_config;
 
 /* context functions */
-int libxl_ctx_alloc(libxl_ctx **pctx, int version, xentoollog_logger *lg);
+int libxl_ctx_alloc(libxl_ctx **pctx, int version,
+                    unsigned flags /* none currently defined */,
+                    xentoollog_logger *lg);
 int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */);
 int libxl_ctx_postfork(libxl_ctx *ctx);
 

diff -r 090e45b2fdca -r ed4bd6824e5e tools/libxl/xl.c

tools/libxl/xl.c +1 -1
@@ -116,7 +116,7 @@ int main(int argc, char **argv)
     logger = xtl_createlogger_stdiostream(stderr, minmsglevel,  0);
     if (!logger) exit(1);
 
-    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, (xentoollog_logger*)logger)) {
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
         fprintf(stderr, "cannot init xl context\n");
         exit(1);
     }

diff -r 090e45b2fdca -r ed4bd6824e5e tools/ocaml/libs/xl/xenlight_stubs.c

tools/ocaml/libs/xl/xenlight_stubs.c +1 -1
@@ -59,7 +59,7 @@ static void log_destroy(struct xentoollo
 	lg.logger.destroy = log_destroy; \
 	lg.logger.progress = NULL; \
 	caml_enter_blocking_section(); \
-	ret = libxl_ctx_alloc(&ctx, LIBXL_VERSION, (struct xentoollog_logger *) &lg); \
+	ret = libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (struct xentoollog_logger *) &lg); \
 	if (ret != 0) \
 		failwith_xl("cannot init context", &lg);
 

diff -r 090e45b2fdca -r ed4bd6824e5e tools/python/xen/lowlevel/xl/xl.c

tools/python/xen/lowlevel/xl/xl.c +1 -1
@@ -687,7 +687,7 @@ PyXl_init(XlObject *self, PyObject *args
         return -1;
     }
 
-    if ( libxl_ctx_alloc(&self->ctx, LIBXL_VERSION,
+    if ( libxl_ctx_alloc(&self->ctx, LIBXL_VERSION, 0,
                 (xentoollog_logger*)self->logger) ) {
         PyErr_SetString(xl_error_obj, "cannot init xl context");
         return -1;
#37 lore raw

libxl stable API work left (Was: Re: [PATCH 00 of 27 v2] libxl: rationalise libxl_device_* APIs)

In reply to Ian Campbell
Quoted 3 lines · Ian Campbell

Along the way I filed some rough edges of the internal implementation of this stuff but my primary concern is to make the public facing API one that we can commit to keeping stable.

Speaking of which, my laundry list for things which need to happen to libxl's API to make it stable contains:
* The stuff this series addresses
* Fixup the event API (IanJ)
* Block script support (IanJ?)
* The topologyinfo datastructure should be a list of tuples, not a tuple of lists.

I've also got:
I've also been wondering what can/should be done about the split between libxl_domain_create_info, libxl_domain_build_info and libxl_device_model_info now that they are all bundled together in libxl_domain_config and not exposed directly in the API (since the related functions became internal, that was before 4.1). It seems like there ought to be scope for collapsing those datastructures somewhat but I'm not sure how yet.

But I think this conflicts with the need to do certain bits asynchronously (primarily for script calling support on create) which I expect will necessitate splitting libxl_domain_create up again.

If you are (or are going to be) a consumer of the libxl API you would be well advised to have a read through libxl.h and make sure you are happy with the rest of it.

Ian.

#4 lore raw

Re: [PATCH 03 of 27 v2] libxl: idl: use "dispose" rather than "destroy" for function to free IDL types

In reply to Ian Campbell

Hello,

This patch doesn't apply clearly on current xen-unstable, gentest.py complains about:

gentest.py +2 -2
@@ -256,8 +256,8 @@ int main(int argc, char **argv)
         f.write("    printf(\"%%s: %%s\\n\", \"%s\", s);\n" % ty.typename)
         f.write("    if (s == NULL) abort();\n")
         f.write("    free(s);\n")
-        if ty.destructor_fn is not None:
-            f.write("    %s(&%s_val);\n" % (ty.destructor_fn, ty.typename))
+        if ty.dispose_fn is not None:
+            f.write("    %s(&%s_val);\n" % (ty.dispose_fn, ty.typename))
         f.write("\n")
 
     f.write("    printf(\"Testing Enumerations\\n\");\n")

And I had to apply the following to gentypes.py and libxltypes.py, since some hunks failed:

tools/libxl/gentypes.py Thu Oct 13 13:19:39 2011 +0200 renamed from tools/libxl/gentypes.py Thu Oct 13 10:47:18 2011 +0100 +3 -3
diff -r eedf352110c5 tools/libxl/gentypes.py
@@ -75,7 +75,7 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
-def libxl_C_type_destroy(ty, v, indent = "    ", parent = None):
+def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
 
     s = ""
     if isinstance(ty, libxltypes.KeyedUnion):
@@ -165,8 +165,8 @@ if __name__ == '__main__':
 
     for ty in types:
         f.write(libxl_C_type_define(ty) + ";\n")
-        if ty.destructor_fn is not None:
-            f.write("void %s(%s);\n" % (ty.destructor_fn, ty.make_arg("p")))
+        if ty.dispose_fn is not None:
+            f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
         if isinstance(ty, libxltypes.Enumeration):
             f.write("const char *%s_to_string(%s);\n" % (ty.typename,

ty.make_arg("p")))

             f.write("int %s_from_string(const char *s, %s);\n" %

(ty.typename, ty.make_arg("e", passby=libxltypes.PASS_BY_REFERENCE))) [51310 refs]
loki# hg qdiff

tools/libxl/gentypes.py Thu Oct 13 13:25:30 2011 +0200 renamed from tools/libxl/gentypes.py Thu Oct 13 10:47:18 2011 +0100 +3 -3
diff -r eedf352110c5 tools/libxl/gentypes.py
@@ -75,7 +75,7 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
-def libxl_C_type_destroy(ty, v, indent = "    ", parent = None):
+def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
 
     s = ""
     if isinstance(ty, libxltypes.KeyedUnion):
@@ -165,8 +165,8 @@ if __name__ == '__main__':
 
     for ty in types:
         f.write(libxl_C_type_define(ty) + ";\n")
-        if ty.destructor_fn is not None:
-            f.write("void %s(%s);\n" % (ty.destructor_fn, ty.make_arg("p")))
+        if ty.dispose_fn is not None:
+            f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
         if isinstance(ty, libxltypes.Enumeration):
             f.write("const char *%s_to_string(%s);\n" % (ty.typename,

ty.make_arg("p")))

             f.write("int %s_from_string(const char *s, %s);\n" %

(ty.typename, ty.make_arg("e", passby=libxltypes.PASS_BY_REFERENCE)))

tools/libxl/libxltypes.py Thu Oct 13 13:25:30 2011 +0200 renamed from tools/libxl/libxltypes.py Thu Oct 13 10:47:18 2011 +0100 +6 -4
diff -r eedf352110c5 tools/libxl/libxltypes.py
@@ -81,14 +81,14 @@ class Type(object):
 class Builtin(Type):
     """Builtin type"""
     def __init__(self, typename, **kwargs):
-        kwargs.setdefault('destructor_fn', None)
-        kwargs.setdefault('autogenerate_destructor', False)
+        kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('autogenerate_dispose_fn', False)
         Type.__init__(self, typename, **kwargs)
 
 class Number(Builtin):
     def __init__(self, ctype, **kwargs):
         kwargs.setdefault('namespace', None)
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('signed', False)
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
@@ -230,7 +230,9 @@ uint16 = UInt(16)
 uint32 = UInt(32)
 uint64 = UInt(64)
 
-string = Builtin("char *", namespace = None, destructor_fn = "free")
+string = Builtin("char *", namespace = None, dispose_fn = "free",
+                  json_fn = "libxl__string_gen_json",
+                  autogenerate_json = False)
 
 class OrderedDict(dict):
     """A dictionary which remembers insertion order.
#5 lore raw

Re: [PATCH 03 of 27 v2] libxl: idl: use "dispose" rather than "destroy" for function to free IDL types

Quoted 4 lines · Roger Pau Monné

Hello,

This patch doesn't apply clearly on current xen-unstable, gentest.py complains about:

Sorry, I forgot to mention that this depends (textually, not functionally) on the "libxl: support json for pretty printing objects" (v2) series which I posted on Friday.

Ian.

Quoted 102 lines
gentest.py +2 -2
@@ -256,8 +256,8 @@ int main(int argc, char **argv)
         f.write("    printf(\"%%s: %%s\\n\", \"%s\", s);\n" % ty.typename)
         f.write("    if (s == NULL) abort();\n")
         f.write("    free(s);\n")
-        if ty.destructor_fn is not None:
-            f.write("    %s(&%s_val);\n" % (ty.destructor_fn, ty.typename))
+        if ty.dispose_fn is not None:
+            f.write("    %s(&%s_val);\n" % (ty.dispose_fn, ty.typename))
         f.write("\n")
 
     f.write("    printf(\"Testing Enumerations\\n\");\n")

And I had to apply the following to gentypes.py and libxltypes.py, since some hunks failed:

diff -r eedf352110c5 tools/libxl/gentypes.py

tools/libxl/gentypes.py Thu Oct 13 13:19:39 2011 +0200 renamed from tools/libxl/gentypes.py Thu Oct 13 10:47:18 2011 +0100 +3 -3
@@ -75,7 +75,7 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
-def libxl_C_type_destroy(ty, v, indent = "    ", parent = None):
+def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
 
     s = ""
     if isinstance(ty, libxltypes.KeyedUnion):
@@ -165,8 +165,8 @@ if __name__ == '__main__':
 
     for ty in types:
         f.write(libxl_C_type_define(ty) + ";\n")
-        if ty.destructor_fn is not None:
-            f.write("void %s(%s);\n" % (ty.destructor_fn, ty.make_arg("p")))
+        if ty.dispose_fn is not None:
+            f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
         if isinstance(ty, libxltypes.Enumeration):
             f.write("const char *%s_to_string(%s);\n" % (ty.typename,
 ty.make_arg("p")))
             f.write("int %s_from_string(const char *s, %s);\n" %

(ty.typename, ty.make_arg("e", passby=libxltypes.PASS_BY_REFERENCE))) [51310 refs]
loki# hg qdiff
diff -r eedf352110c5 tools/libxl/gentypes.py

tools/libxl/gentypes.py Thu Oct 13 13:25:30 2011 +0200 renamed from tools/libxl/gentypes.py Thu Oct 13 10:47:18 2011 +0100 +3 -3
@@ -75,7 +75,7 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
-def libxl_C_type_destroy(ty, v, indent = "    ", parent = None):
+def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
 
     s = ""
     if isinstance(ty, libxltypes.KeyedUnion):
@@ -165,8 +165,8 @@ if __name__ == '__main__':
 
     for ty in types:
         f.write(libxl_C_type_define(ty) + ";\n")
-        if ty.destructor_fn is not None:
-            f.write("void %s(%s);\n" % (ty.destructor_fn, ty.make_arg("p")))
+        if ty.dispose_fn is not None:
+            f.write("void %s(%s);\n" % (ty.dispose_fn, ty.make_arg("p")))
         if isinstance(ty, libxltypes.Enumeration):
             f.write("const char *%s_to_string(%s);\n" % (ty.typename,
 ty.make_arg("p")))
             f.write("int %s_from_string(const char *s, %s);\n" %

(ty.typename, ty.make_arg("e", passby=libxltypes.PASS_BY_REFERENCE))) diff -r eedf352110c5 tools/libxl/libxltypes.py

tools/libxl/libxltypes.py Thu Oct 13 13:25:30 2011 +0200 renamed from tools/libxl/libxltypes.py Thu Oct 13 10:47:18 2011 +0100 +6 -4
@@ -81,14 +81,14 @@ class Type(object):
 class Builtin(Type):
     """Builtin type"""
     def __init__(self, typename, **kwargs):
-        kwargs.setdefault('destructor_fn', None)
-        kwargs.setdefault('autogenerate_destructor', False)
+        kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('autogenerate_dispose_fn', False)
         Type.__init__(self, typename, **kwargs)
 
 class Number(Builtin):
     def __init__(self, ctype, **kwargs):
         kwargs.setdefault('namespace', None)
-        kwargs.setdefault('destructor_fn', None)
+        kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('signed', False)
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
@@ -230,7 +230,9 @@ uint16 = UInt(16)
 uint32 = UInt(32)
 uint64 = UInt(64)
 
-string = Builtin("char *", namespace = None, destructor_fn = "free")
+string = Builtin("char *", namespace = None, dispose_fn = "free",
+                  json_fn = "libxl__string_gen_json",
+                  autogenerate_json = False)
 
 class OrderedDict(dict):
     """A dictionary which remembers insertion order.
#6 lore raw

Re: [PATCH 03 of 27 v2] libxl: idl: use "dispose" rather than "destroy" for function to free IDL types

Quoted 3 lines

Sorry, I forgot to mention that this depends (textually, not functionally) on the "libxl: support json for pretty printing objects" (v2) series which I posted on Friday.

Ok, I was having some trouble applying those patches, I hope that applying JSON ones first will solve it.

#12 lore raw

Re: [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function

In reply to Ian Campbell

Ian Campbell writes ("[Xen-devel] [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function"):

Quoted 10 lines

libxl: do not read f.e. xenstore dir in disk list function

Instead store a duplicate of the "device-type" node in the backend dir and use that instead.

This maintains the invariant that the list function is always "safe".

XXX I'm not sure this is an actual issue. The entries in the frontend device XXX area are writeable by the guest but maybe the solution is to restrict that ?

No, I don't think so. The frontend directory is precisely where the frontend writes things that it wants the backend to be able to read but the backend shouldn't rely on it for its own operation.

Ian.

#13 lore raw

Re: [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function

Quoted 8 lines · Ian Jackson

Ian Campbell writes ("[Xen-devel] [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function"):

Quoted 7 lines

libxl: do not read f.e. xenstore dir in disk list function

Instead store a duplicate of the "device-type" node in the backend dir and use that instead.

This maintains the invariant that the list function is always "safe".

Quoted 7 lines
Quoted 3 lines

XXX I'm not sure this is an actual issue. The entries in the frontend device XXX area are writeable by the guest but maybe the solution is to restrict that ?

No, I don't think so. The frontend directory is precisely where the frontend writes things that it wants the backend to be able to read but the backend shouldn't rely on it for its own operation.

So you agree with the patch and not with my vacillating WRT whether it is necessary?

Ian.

#14 lore raw

Re: [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function

Ian Campbell writes ("Re: [Xen-devel] [PATCH 08 of 27 v2] libxl: do not read f.e. xenstore dir in disk list function"):

Quoted 7 lines · Ian Jackson
Quoted 3 lines

No, I don't think so. The frontend directory is precisely where the frontend writes things that it wants the backend to be able to read but the backend shouldn't rely on it for its own operation.

So you agree with the patch and not with my vacillating WRT whether it is necessary?

Exactly :-).

Ian.

#25 lore raw

Re: [PATCH 18 of 27 v2] libxl: merge libxl__device_del into libxl__device_remove

In reply to Ian Campbell

2011/10/13 Ian Campbell <[email protected]>:

Quoted 105 lines

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1318499565 -3600
# Node ID 95b2f3977d439bba3a01db4a4403a4307f238377 # Parent  f60954a04df9acbddf68af4f2d46c3910f3b0afc
libxl: merge libxl__device_del into libxl__device_remove

Note that the "wait" parameter added to libxl_device_remove is different to the wait paramter previously used by similar functions. In the past not-wait meant forced whereas now in means wait for a graceful shutdown, as opposed to setting off a graceful shutdown but not waiting.

diff -r f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl.c

tools/libxl/libxl.c       Thu Oct 13 10:52:45 2011 +0100 +1 -1
@@ -1075,7 +1075,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
     device.devid            = devid;
     device.kind             = LIBXL__DEVICE_KIND_VBD;
     if (wait)
-        rc = libxl__device_del(&gc, &device);
+        rc = libxl__device_remove(&gc, &device, wait);

    else
        rc = libxl__device_destroy(&gc, &device);
 out_free:

tools/libxl/libxl.c       Thu Oct 13 10:52:45 2011 +0100 +1 -1
@@ -1290,7 +1290,7 @@ int libxl_device_nic_del(libxl_ctx *ctx,
     device.kind             = LIBXL__DEVICE_KIND_VIF;
 
     if (wait)
-        rc = libxl__device_del(&gc, &device);
+        rc = libxl__device_remove(&gc, &device, wait);

    else
        rc = libxl__device_destroy(&gc, &device);

diff -r f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl_device.c

tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +5 -51
@@ -367,57 +367,6 @@ int libxl__device_disk_dev_number(const
     return -1;
  }
 
-int libxl__device_remove(libxl__gc *gc, libxl__device *dev)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    xs_transaction_t t;
-    char *be_path = libxl__device_backend_path(gc, dev);
-    char *state_path = libxl__sprintf(gc, "%s/state", be_path);
-    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
-    int rc = 0;
-
-    if (!state)
-        goto out;
-    if (atoi(state) != 4) {
-        libxl__device_destroy_tapdisk(gc, be_path);
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
-        goto out;
-    }
-
-retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
-    xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
-    xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
-    if (!xs_transaction_end(ctx->xsh, t, 0)) {
-        if (errno == EAGAIN)
-            goto retry_transaction;
-        else {
-            rc = -1;
-            goto out;
-        }
-    }
-
-    xs_watch(ctx->xsh, state_path, be_path);
-    libxl__device_destroy_tapdisk(gc, be_path);
-    rc = 1;
-out:
-    return rc;
-}
-
-int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    char *be_path = libxl__device_backend_path(gc, dev);
-    char *fe_path = libxl__device_frontend_path(gc, dev);
-
-    xs_rm(ctx->xsh, XBT_NULL, be_path);
-    xs_rm(ctx->xsh, XBT_NULL, fe_path);
-
-    libxl__device_destroy_tapdisk(gc, be_path);
-
-    return 0;
-}
-
  static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
  {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -446,6 +395,67 @@ static int wait_for_dev_destroy(libxl__g
     return rc;
  }
 
+/* Returns 0 on success, ERROR_* on fail */
+int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    xs_transaction_t t;

tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 static int wait_for_dev_destroy(libxl__g · line 412

Quoted 10 lines
tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +10 -0
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    int rc = 0;
+
+    if (!state)
+        goto out;
+    if (atoi(state) != 4) {
+        libxl__device_destroy_tapdisk(gc, be_path);
+        xs_rm(ctx->xsh, XBT_NULL, be_path);

I think here we should return something different than 0 (possibly 1?) so the number of watches (n_watches) is not increased.

Quoted 15 lines
tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +15 -0
+        goto out;
+    }
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
+    xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
+    if (!xs_transaction_end(ctx->xsh, t, 0)) {
+        if (errno == EAGAIN)
+            goto retry_transaction;
+        else {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }

tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 static int wait_for_dev_destroy(libxl__g · line 437

Quoted 10 lines
tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +10 -0
+
+    xs_watch(ctx->xsh, state_path, be_path);
+    libxl__device_destroy_tapdisk(gc, be_path);
+
+    if (wait) {
+        struct timeval tv;
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        (void)wait_for_dev_destroy(gc, &tv);
+        xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));

Here we should check if the device is removed correctly or not, so that the number of watches is not increased:

if(wait_for_dev_destroy(gc, &tv) != 0) /* device destroyed */

    rc = 1;

tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 static int wait_for_dev_destroy(libxl__g · line 440

Quoted 3 lines
tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +3 -0
+    }
+
+    rc = 0;

This should also be removed, since rc is initialized to 0 already.

Quoted 88 lines
tools/libxl/libxl_device.c        Thu Oct 13 10:52:45 2011 +0100 +20 -25
+out:
+    return rc;
+}
+
+int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *fe_path = libxl__device_frontend_path(gc, dev);
+
+    xs_rm(ctx->xsh, XBT_NULL, be_path);
+    xs_rm(ctx->xsh, XBT_NULL, fe_path);
+
+    libxl__device_destroy_tapdisk(gc, be_path);
+
+    return 0;
+}
+
  int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force)
  {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -485,7 +495,7 @@ int libxl__devices_destroy(libxl__gc *gc
                 if (force) {
                     libxl__device_destroy(gc, &dev);
                 } else {
-                    if (libxl__device_remove(gc, &dev) > 0)
+                    if (libxl__device_remove(gc, &dev, 0) == 0)
                         n_watches++;
                 }
             }
@@ -504,7 +514,7 @@ int libxl__devices_destroy(libxl__gc *gc
         if (force) {
             libxl__device_destroy(gc, &dev);
         } else {
-            if (libxl__device_remove(gc, &dev) > 0)
+            if (libxl__device_remove(gc, &dev, 0) == 0)
                 n_watches++;
         }
     }
@@ -530,29 +540,6 @@ out:
     return 0;
  }
 
-int libxl__device_del(libxl__gc *gc, libxl__device *dev)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    struct timeval tv;
-    int rc;
-
-    rc = libxl__device_remove(gc, dev);
-    if (rc == -1) {
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
-    tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
-    tv.tv_usec = 0;
-    (void)wait_for_dev_destroy(gc, &tv);
-
-    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
-    rc = 0;
-
-out:
-    return rc;
-}
-
  int libxl__wait_for_device_model(libxl__gc *gc,
                                  uint32_t domid, char *state,
                                  libxl__spawn_starting *spawning,

diff -r f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl_internal.h

tools/libxl/libxl_internal.h      Thu Oct 13 10:52:45 2011 +0100 +1 -2
@@ -242,8 +242,7 @@ _hidden char *libxl__device_backend_path
  _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
  _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
-_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
-_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
+_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait);
  _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
  _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
  _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#26 lore raw

Re: [PATCH 18 of 27 v2] libxl: merge libxl__device_del into libxl__device_remove

Quoted 14 lines · Roger Pau Monné
Quoted 14 lines
tools/libxl/libxl_device.c +10 -0
@@ -446,6 +395,67 @@ static int wait_for_dev_destroy(libxl__g
    return rc;
 }
 
+/* Returns 0 on success, ERROR_* on fail */
+int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    xs_transaction_t t;
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *state_path = libxl__sprintf(gc, "%s/state", be_path);
+    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    int rc = 0;
+
Quoted 8 lines

tools/libxl/libxl_device.c static int wait_for_dev_destroy(libxl__g · line 412

Quoted 5 lines
tools/libxl/libxl_device.c +5 -0
+    if (!state)
+        goto out;
+    if (atoi(state) != 4) {
+        libxl__device_destroy_tapdisk(gc, be_path);
+        xs_rm(ctx->xsh, XBT_NULL, be_path);

I think here we should return something different than 0 (possibly 1?) so the number of watches (n_watches) is not increased.

Yes, I think you are correct. We need to distinguish 3 cases: error, success--event pending and success--no event pending. But this patch only considers 2 of them.

Previously there was a model of returning the number of events which are pending which I think makes sense (I understand what that was for now ;-)), so the returns would become ERROR_* (all -ve), 0 (success--no event pending) and 1 (success--event pending).

Quoted 29 lines
Quoted 15 lines
tools/libxl/libxl_device.c +15 -0
+        goto out;
+    }
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", strlen("0"));
+    xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
+    if (!xs_transaction_end(ctx->xsh, t, 0)) {
+        if (errno == EAGAIN)
+            goto retry_transaction;
+        else {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }

tools/libxl/libxl_device.c static int wait_for_dev_destroy(libxl__g · line 437

Quoted 10 lines
tools/libxl/libxl_device.c +10 -0
+
+    xs_watch(ctx->xsh, state_path, be_path);
+    libxl__device_destroy_tapdisk(gc, be_path);
+
+    if (wait) {
+        struct timeval tv;
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        (void)wait_for_dev_destroy(gc, &tv);
+        xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));

Here we should check if the device is removed correctly or not, so that the number of watches is not increased:

Quoted 2 lines

if(wait_for_dev_destroy(gc, &tv) != 0) /* device destroyed */

    rc = 1;

Agreed.

Quoted 5 lines

tools/libxl/libxl_device.c static int wait_for_dev_destroy(libxl__g · line 440

Quoted 3 lines
tools/libxl/libxl_device.c +3 -0
+    }
+
+    rc = 0;

This should also be removed, since rc is initialized to 0 already.

Right. Thanks for your review.

Ian.

#38 lore raw

PATCH 1/1 libxl: fix device removal handling of wait_for_dev_destroy

In reply to Ian Campbell

# HG changeset patch
# User Roger Pau Monne <[email protected]> # Date 1318942350 -7200
# Node ID 973022eb35ed3c2e58980d8c4482721d4e1d7bab # Parent c9c228ec718a4c2d22b58ce306f269868af0d12f
libxl: fix device removal handling of wait_for_dev_destroy

This patch is a fix for Ian Campbell's "libxl: rationalise libxl_device_* APIs" and addresses the problem of not handling the return value from destroy functions correctly. The return value can represent three different options, the device is removed correctly, the device is busy or and error occurred.

tools/libxl/libxl_device.c +41 -18
diff -r c9c228ec718a -r 973022eb35ed tools/libxl/libxl_device.c
@@ -367,6 +367,10 @@ int libxl__device_disk_dev_number(const 
     return -1;
 }
 
+/*
+ * Returns 0 on succesful device removal, 1 if a timeout
+ * happenned and ERROR_* if there was an error
+ */
 static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -375,27 +379,47 @@ static int wait_for_dev_destroy(libxl__g
     fd_set rfds;
     char **l1 = NULL;
 
+start:
     rc = 1;
     nfds = xs_fileno(ctx->xsh) + 1;
     FD_ZERO(&rfds);
     FD_SET(xs_fileno(ctx->xsh), &rfds);
-    if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
-        l1 = xs_read_watch(ctx->xsh, &n);
-        if (l1 != NULL) {
-            char *state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
-            if (!state || atoi(state) == 6) {
-                xs_unwatch(ctx->xsh, l1[0], l1[1]);
-                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Destroyed device backend at %s", l1[XS_WATCH_TOKEN]);
-                rc = 0;
+    switch (select(nfds, &rfds, NULL, NULL, tv)) {
+        case -1:
+            if (errno == EINTR)
+                goto start;
+            rc = ERROR_FAIL;
+            break;
+        case 0:
+            rc = 1;
+            break;
+        case 1:
+            l1 = xs_read_watch(ctx->xsh, &n);
+            if (l1 != NULL) {
+                char *state = libxl__xs_read(gc, XBT_NULL,
+                                             l1[XS_WATCH_PATH]);
+                if (!state || atoi(state) == 6) {
+                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                               "Destroyed device backend at %s",
+                               l1[XS_WATCH_TOKEN]);
+                    rc = 0;
+                }
+                free(l1);
+            } else {
+                rc = ERROR_FAIL;
             }
-            free(l1);
-        }
+            break;
     }
     return rc;
 }
 
-/* Returns 0 on success, ERROR_* on fail */
+/* 
+ * Returns 0 on success, ERROR_* on fail and 1 if there's an event
+ * pending (a device has been added to a watch to wait for
+ * disconnection)
+ */
 int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -428,16 +452,15 @@ retry_transaction:
 
     xs_watch(ctx->xsh, state_path, be_path);
     libxl__device_destroy_tapdisk(gc, be_path);
+    rc = 1;
 
     if (wait) {
         struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        (void)wait_for_dev_destroy(gc, &tv);
+        rc = wait_for_dev_destroy(gc, &tv);
         xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
     }
-
-    rc = 0;
 out:
     return rc;
 }
@@ -496,7 +519,7 @@ int libxl__devices_destroy(libxl__gc *gc
                 if (force) {
                     libxl__device_destroy(gc, &dev);
                 } else {
-                    if (libxl__device_remove(gc, &dev, 0) == 0)
+                    if (libxl__device_remove(gc, &dev, 0) == 1)
                         n_watches++;
                 }
             }
@@ -515,7 +538,7 @@ int libxl__devices_destroy(libxl__gc *gc
         if (force) {
             libxl__device_destroy(gc, &dev);
         } else {
-            if (libxl__device_remove(gc, &dev, 0) == 0)
+            if (libxl__device_remove(gc, &dev, 0) == 1)
                 n_watches++;
         }
     }
@@ -531,7 +554,7 @@ int libxl__devices_destroy(libxl__gc *gc
         tv.tv_usec = 0;
         while (n_watches > 0) {
             if (wait_for_dev_destroy(gc, &tv)) {
-                break;
+                continue;
             } else {
                 n_watches--;
             }