Daniel De Graaf
2012-May-07 19:15 UTC
[PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro
The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the expression CTX->osevent_in_hook-- (usually 1) instead of the value of the function call it made. Change the macro to a statement including the return variable to fix this. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- tools/libxl/libxl_event.c | 37 +++++++++++++++++++++---------------- 1 files changed, 21 insertions(+), 16 deletions(-) diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c index 638b9ab..6aced19 100644 --- a/tools/libxl/libxl_event.c +++ b/tools/libxl/libxl_event.c @@ -27,18 +27,23 @@ * these macros, with the ctx locked. Likewise all the "occurred" * entrypoints from the application should assert(!in_hook); */ -#define OSEVENT_HOOK_INTERN(defval, hookname, ...) \ - (CTX->osevent_hooks \ - ? (CTX->osevent_in_hook++, \ - CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__), \ - CTX->osevent_in_hook--) \ - : defval) - -#define OSEVENT_HOOK(hookname,...) \ - OSEVENT_HOOK_INTERN(0, hookname, __VA_ARGS__) - -#define OSEVENT_HOOK_VOID(hookname,...) \ - OSEVENT_HOOK_INTERN((void)0, hookname, __VA_ARGS__) +#define OSEVENT_HOOK_RET(rc, hookname,...) do { \ + if (CTX->osevent_hooks) { \ + CTX->osevent_in_hook++; \ + rc = CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ + CTX->osevent_in_hook--; \ + } else { \ + rc = 0; \ + } \ +} while (0) + +#define OSEVENT_HOOK_VOID(hookname,...) do { \ + if (CTX->osevent_hooks) { \ + CTX->osevent_in_hook++; \ + CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ + CTX->osevent_in_hook--; \ + } \ +} while (0) /* * fd events @@ -54,7 +59,7 @@ int libxl__ev_fd_register(libxl__gc *gc, libxl__ev_fd *ev, CTX_LOCK; - rc = OSEVENT_HOOK(fd_register, fd, &ev->for_app_reg, events, ev); + OSEVENT_HOOK_RET(rc, fd_register, fd, &ev->for_app_reg, events, ev); if (rc) goto out; ev->fd = fd; @@ -77,7 +82,7 @@ int libxl__ev_fd_modify(libxl__gc *gc, libxl__ev_fd *ev, short events) CTX_LOCK; assert(libxl__ev_fd_isregistered(ev)); - rc = OSEVENT_HOOK(fd_modify, ev->fd, &ev->for_app_reg, events); + OSEVENT_HOOK_RET(rc, fd_modify, ev->fd, &ev->for_app_reg, events); if (rc) goto out; ev->events = events; @@ -147,7 +152,7 @@ static int time_register_finite(libxl__gc *gc, libxl__ev_time *ev, { int rc; - rc = OSEVENT_HOOK(timeout_register, &ev->for_app_reg, abs, ev); + OSEVENT_HOOK_RET(rc, timeout_register, &ev->for_app_reg, abs, ev); if (rc) return rc; ev->infinite = 0; @@ -226,7 +231,7 @@ int libxl__ev_time_modify_abs(libxl__gc *gc, libxl__ev_time *ev, rc = time_register_finite(gc, ev, abs); if (rc) goto out; } else { - rc = OSEVENT_HOOK(timeout_modify, &ev->for_app_reg, abs); + OSEVENT_HOOK_RET(rc, timeout_modify, &ev->for_app_reg, abs); if (rc) goto out; LIBXL_TAILQ_REMOVE(&CTX->etimes, ev, entry); -- 1.7.7.6
Ian Jackson
2012-May-09 13:19 UTC
Re: [PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro
Daniel De Graaf writes ("[PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro"):> The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the > expression CTX->osevent_in_hook-- (usually 1) instead of the value of > the function call it made. Change the macro to a statement including the > return variable to fix this.Well spotted, thanks. But I think it would be better to use the GCC statement expression syntax to avoid changing the call sites ? Ian.
Ian Jackson
2012-May-09 13:20 UTC
Re: [PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro
Daniel De Graaf writes ("[PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro"):> The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the > expression CTX->osevent_in_hook-- (usually 1) instead of the value of > the function call it made. Change the macro to a statement including the > return variable to fix this.Also, does this mean that you''re using this in anger, or did you just happen to spot it while perusing the code ? Thanks, Ian.
Daniel De Graaf
2012-May-09 13:38 UTC
[PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro
On 05/09/2012 09:19 AM, Ian Jackson wrote:> Daniel De Graaf writes ("[PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro"): >> The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the >> expression CTX->osevent_in_hook-- (usually 1) instead of the value of >> the function call it made. Change the macro to a statement including the >> return variable to fix this. > > Well spotted, thanks. But I think it would be better to use the GCC > statement expression syntax to avoid changing the call sites ? > > Ian. >Agreed, using the statement expression syntax seems cleaner here. ------8<------------------------------ The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the expression CTX->osevent_in_hook-- (usually 1) instead of the value of the function call it made. Fix the macro to return the proper value. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- tools/libxl/libxl_event.c | 31 +++++++++++++++++++------------ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c index 638b9ab..bee0e27 100644 --- a/tools/libxl/libxl_event.c +++ b/tools/libxl/libxl_event.c @@ -27,18 +27,25 @@ * these macros, with the ctx locked. Likewise all the "occurred" * entrypoints from the application should assert(!in_hook); */ -#define OSEVENT_HOOK_INTERN(defval, hookname, ...) \ - (CTX->osevent_hooks \ - ? (CTX->osevent_in_hook++, \ - CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__), \ - CTX->osevent_in_hook--) \ - : defval) - -#define OSEVENT_HOOK(hookname,...) \ - OSEVENT_HOOK_INTERN(0, hookname, __VA_ARGS__) - -#define OSEVENT_HOOK_VOID(hookname,...) \ - OSEVENT_HOOK_INTERN((void)0, hookname, __VA_ARGS__) +#define OSEVENT_HOOK(hookname,...) ({ \ + int rc; \ + if (CTX->osevent_hooks) { \ + CTX->osevent_in_hook++; \ + rc = CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ + CTX->osevent_in_hook--; \ + } else { \ + rc = 0; \ + } \ + rc; \ +}) + +#define OSEVENT_HOOK_VOID(hookname,...) do { \ + if (CTX->osevent_hooks) { \ + CTX->osevent_in_hook++; \ + CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ + CTX->osevent_in_hook--; \ + } \ +} while (0) /* * fd events
Daniel De Graaf
2012-May-09 13:50 UTC
Re: [PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro
On 05/09/2012 09:20 AM, Ian Jackson wrote:> Daniel De Graaf writes ("[PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro"): >> The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the >> expression CTX->osevent_in_hook-- (usually 1) instead of the value of >> the function call it made. Change the macro to a statement including the >> return variable to fix this. > > Also, does this mean that you''re using this in anger, or did you just > happen to spot it while perusing the code ? > > Thanks, > Ian. >I am using this functionality in a locally-patched libvirt where I have replaced libxl 4.1 support with 4.2. I don''t think the changes I made are suitable for upstream as I assume libvirt will want to continue to support libxl 4.1 in addition to 4.2 (and my patch discards the 4.1 support). -- Daniel De Graaf National Security Agency
Ian Jackson
2012-May-09 15:32 UTC
Re: [PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro
Daniel De Graaf writes ("[PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro"):> Agreed, using the statement expression syntax seems cleaner here....> +#define OSEVENT_HOOK(hookname,...) ({ \ > + int rc; \ > + if (CTX->osevent_hooks) { \ > + CTX->osevent_in_hook++; \ > + rc = CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ > + CTX->osevent_in_hook--; \ > + } else { \ > + rc = 0; \ > + } \ > + rc; \ > +})Thanks. However, you need to use a different variable name to "rc". "rc" might theoretically be present int __VA_ARGS__ in which case this will go wrong. Also if we ever enable -Wshadow, this construction will cause a warning. I suggest int osevent_hook_rc; You could preserve the unification of the two macros by having #define OSEVENT_HOOK(hookname,...) \ OSEVENT_HOOK_INTERN(osevent_hook_rc =, /*empty*/, hookname, __VA_ARGS__) #define OSEVENT_HOOK_VOID(hookname,...) \ OSEVENT_HOOK_INTERN(/*empty*/, (void), hookname, __VA_ARGS__) or some such. I don''t know whether you like that idea; if you do please go ahead and do it. If not then I''m happy to take the patch which duplicates the macro. Ian.
Ian Jackson
2012-May-09 15:33 UTC
Re: [PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro
Daniel De Graaf writes ("Re: [PATCH] libxl: Fix incorrect return of OSEVENT_HOOK macro"):> I am using this functionality in a locally-patched libvirt where I have > replaced libxl 4.1 support with 4.2. I don''t think the changes I made are > suitable for upstream as I assume libvirt will want to continue to support > libxl 4.1 in addition to 4.2 (and my patch discards the 4.1 support).Right. But we might like to see them anyway; they might provide a basis for a more-compatibility-respecting 4.2 port. Thanks! Ian.
Daniel De Graaf
2012-May-09 16:42 UTC
Re: [PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro
On 05/09/2012 11:32 AM, Ian Jackson wrote:> Daniel De Graaf writes ("[PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro"): >> Agreed, using the statement expression syntax seems cleaner here. > ... >> +#define OSEVENT_HOOK(hookname,...) ({ \ >> + int rc; \ >> + if (CTX->osevent_hooks) { \ >> + CTX->osevent_in_hook++; \ >> + rc = CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ >> + CTX->osevent_in_hook--; \ >> + } else { \ >> + rc = 0; \ >> + } \ >> + rc; \ >> +}) > > Thanks. However, you need to use a different variable name to "rc". > "rc" might theoretically be present int __VA_ARGS__ in which case this > will go wrong. Also if we ever enable -Wshadow, this construction > will cause a warning. I suggest int osevent_hook_rc; > > You could preserve the unification of the two macros by having > > #define OSEVENT_HOOK(hookname,...) \ > OSEVENT_HOOK_INTERN(osevent_hook_rc =, /*empty*/, hookname, __VA_ARGS__) > > #define OSEVENT_HOOK_VOID(hookname,...) \ > OSEVENT_HOOK_INTERN(/*empty*/, (void), hookname, __VA_ARGS__) > > or some such. I don''t know whether you like that idea; if you do > please go ahead and do it. If not then I''m happy to take the patch > which duplicates the macro. > > Ian. >I like that method (or this slight tweak to it): -----------8<------------------------- The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the expression CTX->osevent_in_hook-- (usually 1) instead of the value of the function call it made. Fix the macro to return the proper value. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- tools/libxl/libxl_event.c | 28 ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 deletions(-) diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c index 638b9ab..a31aec7 100644 --- a/tools/libxl/libxl_event.c +++ b/tools/libxl/libxl_event.c @@ -27,18 +27,22 @@ * these macros, with the ctx locked. Likewise all the "occurred" * entrypoints from the application should assert(!in_hook); */ -#define OSEVENT_HOOK_INTERN(defval, hookname, ...) \ - (CTX->osevent_hooks \ - ? (CTX->osevent_in_hook++, \ - CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__), \ - CTX->osevent_in_hook--) \ - : defval) - -#define OSEVENT_HOOK(hookname,...) \ - OSEVENT_HOOK_INTERN(0, hookname, __VA_ARGS__) - -#define OSEVENT_HOOK_VOID(hookname,...) \ - OSEVENT_HOOK_INTERN((void)0, hookname, __VA_ARGS__) +#define OSEVENT_HOOK_INTERN(retval, hookname, ...) do { \ + if (CTX->osevent_hooks) { \ + CTX->osevent_in_hook++; \ + retval CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__); \ + CTX->osevent_in_hook--; \ + } \ +} while (0) + +#define OSEVENT_HOOK(hookname, ...) ({ \ + int osevent_hook_rc = 0; \ + OSEVENT_HOOK_INTERN(osevent_hook_rc = , hookname, __VA_ARGS__); \ + osevent_hook_rc; \ +}) + +#define OSEVENT_HOOK_VOID(hookname, ...) \ + OSEVENT_HOOK_INTERN(/* void */, hookname, __VA_ARGS__) /* * fd events
This patch changes libxl to use the interface in Xen 4.2. It is provided as an example, not intended to go in to libvirt as-is since it removes all support for libxl from Xen 4.1. It also still has some cruft (extra void casts on parameters) and the device model info population is not written. It has been tested with simple domain create/destroy. --- src/libxl/libxl_conf.c | 134 +++++++++------ src/libxl/libxl_conf.h | 8 +- src/libxl/libxl_driver.c | 430 ++++++++++++++++++++++++++-------------------- 3 files changed, 326 insertions(+), 246 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 62621f1..c5b5561 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -41,7 +41,7 @@ #include "capabilities.h" #include "libxl_driver.h" #include "libxl_conf.h" - +#include "libxl_utils.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -358,18 +358,29 @@ libxlMakeCapabilitiesInternal(const char *hostmachine, } static int -libxlMakeDomCreateInfo(virDomainDefPtr def, libxl_domain_create_info *c_info) +libxlMakeDomCreateInfo(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_create_info *c_info) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - libxl_init_create_info(c_info); + libxl_domain_create_info_init(c_info); + + if (STREQ(def->os.type, "hvm")) + c_info->type = LIBXL_DOMAIN_TYPE_HVM; + else + c_info->type = LIBXL_DOMAIN_TYPE_PV; - c_info->hvm = STREQ(def->os.type, "hvm"); if ((c_info->name = strdup(def->name)) == NULL) { virReportOOMError(); goto error; } + if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) { + if (libxl_flask_context_to_sid(driver->ctx, def->seclabel.label, strlen(def->seclabel.label), &c_info->ssidref)) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to resolve security label ''%s''"), def->seclabel.label); + } + } + virUUIDFormat(def->uuid, uuidstr); if (libxl_uuid_from_string(&c_info->uuid, uuidstr) ) { libxlError(VIR_ERR_INTERNAL_ERROR, @@ -380,7 +391,7 @@ libxlMakeDomCreateInfo(virDomainDefPtr def, libxl_domain_create_info *c_info) return 0; error: - libxl_domain_create_info_destroy(c_info); + libxl_domain_create_info_dispose(c_info); return -1; } @@ -403,9 +414,9 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) return -1; } - libxl_init_build_info(b_info, &d_config->c_info); + libxl_domain_build_info_init(b_info); - b_info->hvm = hvm; + b_info->type = hvm ? LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PV; b_info->max_vcpus = def->maxvcpus; if (def->vcpus == 32) b_info->cur_vcpus = (uint32_t) -1; @@ -424,16 +435,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) b_info->tsc_mode = 1; } } + b_info->sched_params.weight = 1000; b_info->max_memkb = def->mem.max_balloon; b_info->target_memkb = def->mem.cur_balloon; if (hvm) { - b_info->u.hvm.pae = def->features & (1 << VIR_DOMAIN_FEATURE_PAE); - b_info->u.hvm.apic = def->features & (1 << VIR_DOMAIN_FEATURE_APIC); - b_info->u.hvm.acpi = def->features & (1 << VIR_DOMAIN_FEATURE_ACPI); + libxl_defbool_set(&b_info->u.hvm.pae, def->features & (1 << VIR_DOMAIN_FEATURE_PAE)); + libxl_defbool_set(&b_info->u.hvm.apic, def->features & (1 << VIR_DOMAIN_FEATURE_APIC)); + libxl_defbool_set(&b_info->u.hvm.acpi, def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)); for (i = 0; i < def->clock.ntimers; i++) { if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET && def->clock.timers[i]->present == 1) { - b_info->u.hvm.hpet = 1; + libxl_defbool_set(&b_info->u.hvm.hpet, 1); } } @@ -454,7 +466,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) } } if (def->os.bootloaderArgs) { - if ((b_info->u.pv.bootloader_args = strdup(def->os.bootloaderArgs)) == NULL) { + // XXX may need to split these arguments on a delimiter + b_info->u.pv.bootloader_args = malloc(sizeof(char*)); + if (b_info->u.pv.bootloader_args == NULL) { + virReportOOMError(); + goto error; + } + b_info->u.pv.bootloader_args[0] = strdup(def->os.bootloaderArgs); + if (b_info->u.pv.bootloader_args[0] == NULL) { virReportOOMError(); goto error; } @@ -467,8 +486,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) } if (def->os.kernel) { /* libxl_init_build_info() sets kernel.path = strdup("hvmloader") */ - VIR_FREE(b_info->kernel.path); - if ((b_info->kernel.path = strdup(def->os.kernel)) == NULL) { + VIR_FREE(b_info->u.pv.kernel.path); + if ((b_info->u.pv.kernel.path = strdup(def->os.kernel)) == NULL) { virReportOOMError(); goto error; } @@ -484,7 +503,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) return 0; error: - libxl_domain_build_info_destroy(b_info); + libxl_domain_build_info_dispose(b_info); return -1; } @@ -507,30 +526,30 @@ libxlMakeDisk(virDomainDefPtr def, virDomainDiskDefPtr l_disk, STREQ(l_disk->driverName, "tap2")) { if (l_disk->driverType) { if (STREQ(l_disk->driverType, "qcow")) { - x_disk->format = DISK_FORMAT_QCOW; - x_disk->backend = DISK_BACKEND_QDISK; + x_disk->format = LIBXL_DISK_FORMAT_QCOW; + x_disk->backend = LIBXL_DISK_BACKEND_QDISK; } else if (STREQ(l_disk->driverType, "qcow2")) { - x_disk->format = DISK_FORMAT_QCOW2; - x_disk->backend = DISK_BACKEND_QDISK; + x_disk->format = LIBXL_DISK_FORMAT_QCOW2; + x_disk->backend = LIBXL_DISK_BACKEND_QDISK; } else if (STREQ(l_disk->driverType, "vhd")) { - x_disk->format = DISK_FORMAT_VHD; - x_disk->backend = DISK_BACKEND_TAP; + x_disk->format = LIBXL_DISK_FORMAT_VHD; + x_disk->backend = LIBXL_DISK_BACKEND_TAP; } else if (STREQ(l_disk->driverType, "aio") || STREQ(l_disk->driverType, "raw")) { - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; + x_disk->format = LIBXL_DISK_FORMAT_RAW; + x_disk->backend = LIBXL_DISK_BACKEND_TAP; } } else { /* No subtype specified, default to raw/tap */ - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; + x_disk->format = LIBXL_DISK_FORMAT_RAW; + x_disk->backend = LIBXL_DISK_BACKEND_TAP; } } else if (STREQ(l_disk->driverName, "file")) { - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; + x_disk->format = LIBXL_DISK_FORMAT_RAW; + x_disk->backend = LIBXL_DISK_BACKEND_TAP; } else if (STREQ(l_disk->driverName, "phy")) { - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_PHY; + x_disk->format = LIBXL_DISK_FORMAT_RAW; + x_disk->backend = LIBXL_DISK_BACKEND_PHY; } else { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxenlight does not support disk driver %s"), @@ -538,13 +557,14 @@ libxlMakeDisk(virDomainDefPtr def, virDomainDiskDefPtr l_disk, return -1; } } else { - /* No driverName - default to raw/tap?? */ - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; + + /* No driverName - default to raw/phy?? */ + x_disk->format = LIBXL_DISK_FORMAT_RAW; + x_disk->backend = LIBXL_DISK_BACKEND_PHY; } - /* How to set unpluggable? */ - x_disk->unpluggable = 1; + /* XXX is this right? */ + x_disk->removable = 1; x_disk->readwrite = !l_disk->readonly; x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; if (l_disk->transient) { @@ -553,7 +573,7 @@ libxlMakeDisk(virDomainDefPtr def, virDomainDiskDefPtr l_disk, return -1; } - x_disk->domid = def->id; + (void)def->id; return 0; } @@ -583,7 +603,7 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) error: for (i = 0; i < ndisks; i++) - libxl_device_disk_destroy(&x_disks[i]); + libxl_device_disk_dispose(&x_disks[i]); VIR_FREE(x_disks); return -1; } @@ -595,7 +615,7 @@ libxlMakeNic(virDomainDefPtr def, virDomainNetDefPtr l_nic, // TODO: Where is mtu stored? //x_nics[i].mtu = 1492; - x_nic->domid = def->id; + (void)def->id; memcpy(x_nic->mac, l_nic->mac, sizeof(libxl_mac)); if (l_nic->model && !STREQ(l_nic->model, "netfront")) { @@ -603,9 +623,9 @@ libxlMakeNic(virDomainDefPtr def, virDomainNetDefPtr l_nic, virReportOOMError(); return -1; } - x_nic->nictype = NICTYPE_IOEMU; + x_nic->nictype = LIBXL_NIC_TYPE_IOEMU; } else { - x_nic->nictype = NICTYPE_VIF; + x_nic->nictype = LIBXL_NIC_TYPE_VIF; } if (l_nic->ifname && (x_nic->ifname = strdup(l_nic->ifname)) == NULL) { @@ -663,7 +683,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) error: for (i = 0; i < nnics; i++) - libxl_device_nic_destroy(&x_nics[i]); + libxl_device_nic_dispose(&x_nics[i]); VIR_FREE(x_nics); return -1; } @@ -677,23 +697,23 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainDefPtr def, switch (l_vfb->type) { case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - x_vfb->sdl = 1; + libxl_defbool_set(&x_vfb->sdl.enable, 1); if (l_vfb->data.sdl.display && - (x_vfb->display = strdup(l_vfb->data.sdl.display)) == NULL) { + (x_vfb->sdl.display = strdup(l_vfb->data.sdl.display)) == NULL) { virReportOOMError(); return -1; } if (l_vfb->data.sdl.xauth && - (x_vfb->xauthority + (x_vfb->sdl.xauthority strdup(l_vfb->data.sdl.xauth)) == NULL) { virReportOOMError(); return -1; } break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - x_vfb->vnc = 1; + libxl_defbool_set(&x_vfb->vnc.enable, 1); /* driver handles selection of free port */ - x_vfb->vncunused = 0; + libxl_defbool_set(&x_vfb->vnc.findunused, 0); if (l_vfb->data.vnc.autoport) { port = libxlNextFreeVncPort(driver, LIBXL_VNC_PORT_MIN); if (port < 0) { @@ -703,13 +723,13 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainDefPtr def, } l_vfb->data.vnc.port = port; } - x_vfb->vncdisplay = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; + x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0); if (listenAddr) { /* libxl_device_vfb_init() does strdup("127.0.0.1") */ - VIR_FREE(x_vfb->vnclisten); - if ((x_vfb->vnclisten = strdup(listenAddr)) == NULL) { + VIR_FREE(x_vfb->vnc.listen); + if ((x_vfb->vnc.listen = strdup(listenAddr)) == NULL) { virReportOOMError(); return -1; } @@ -722,7 +742,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainDefPtr def, } break; } - x_vfb->domid = def->id; + (void) def->id; return 0; } @@ -750,8 +770,8 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, } for (i = 0; i < nvfbs; i++) { - libxl_device_vfb_init(&x_vfbs[i], i); - libxl_device_vkb_init(&x_vkbs[i], i); + libxl_device_vfb_init(&x_vfbs[i]); + libxl_device_vkb_init(&x_vkbs[i]); if (libxlMakeVfb(driver, def, l_vfbs[i], &x_vfbs[i]) < 0) goto error; @@ -765,14 +785,15 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, error: for (i = 0; i < nvfbs; i++) { - libxl_device_vfb_destroy(&x_vfbs[i]); - libxl_device_vkb_destroy(&x_vkbs[i]); + libxl_device_vfb_dispose(&x_vfbs[i]); + libxl_device_vkb_dispose(&x_vkbs[i]); } VIR_FREE(x_vfbs); VIR_FREE(x_vkbs); return -1; } +#if 0 static int libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) { @@ -906,6 +927,7 @@ error: libxl_device_model_info_destroy(dm_info); return -1; } +#endif virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx) @@ -940,7 +962,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) { - if (libxlMakeDomCreateInfo(def, &d_config->c_info) < 0) + if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0) return -1; if (libxlMakeDomBuildInfo(def, d_config) < 0) { @@ -959,9 +981,11 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, goto error; } +#if 0 if (libxlMakeDeviceModelInfo(def, d_config) < 0) { goto error; } +#endif d_config->on_reboot = def->onReboot; d_config->on_poweroff = def->onPoweroff; @@ -970,6 +994,6 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, return 0; error: - libxl_domain_config_destroy(d_config); + libxl_domain_config_dispose(d_config); return -1; } diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 2820afb..73ab733 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -58,7 +58,7 @@ struct _libxlDriverPrivate { FILE *logger_file; xentoollog_logger *logger; /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */ - libxl_ctx ctx; + libxl_ctx *ctx; virBitmapPtr reservedVNCPorts; virDomainObjList domains; @@ -77,10 +77,8 @@ typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate; typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr; struct _libxlDomainObjPrivate { /* per domain libxl ctx */ - libxl_ctx ctx; - libxl_waiter *dWaiter; - int waiterFD; - int eventHdl; + libxl_ctx *ctx; + libxl_evgen_domain_death *deathW; }; # define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 45bf1f8..8656f35 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -40,6 +40,7 @@ #include "memory.h" #include "uuid.h" #include "command.h" +#include "libxl.h" #include "libxl_driver.h" #include "libxl_conf.h" #include "xen_xm.h" @@ -79,6 +80,138 @@ libxlDriverUnlock(libxlDriverPrivatePtr driver) virMutexUnlock(&driver->lock); } +struct libxl_osevent_hook_fdinfo { + libxlDomainObjPrivatePtr priv; + void *xl_priv; + int watch; +}; + +static void cb_fd_event(int watch, int fd, int vir_events, void *fdinfo) +{ + struct libxl_osevent_hook_fdinfo *info = fdinfo; + int events = 0; + (void)watch; (void)fd; + if (vir_events & VIR_EVENT_HANDLE_READABLE) + events |= POLLIN; + if (vir_events & VIR_EVENT_HANDLE_WRITABLE) + events |= POLLOUT; + if (vir_events & VIR_EVENT_HANDLE_ERROR) + events |= POLLERR; + if (vir_events & VIR_EVENT_HANDLE_HANGUP) + events |= POLLHUP; + libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events); +} + +static void free_fdinfo(void *opaque) +{ + free(opaque); +} + +static int evhook_fd_register(void *priv, int fd, void **hndp, short events, void *xl_priv) +{ + int vir_events = VIR_EVENT_HANDLE_ERROR; + struct libxl_osevent_hook_fdinfo *fdinfo; + fdinfo = malloc(sizeof(fdinfo)); + fdinfo->priv = priv; + fdinfo->xl_priv = xl_priv; + *hndp = fdinfo; + + if (events & POLLIN) + vir_events |= VIR_EVENT_HANDLE_READABLE; + if (events & POLLOUT) + vir_events |= VIR_EVENT_HANDLE_WRITABLE; + fdinfo->watch = virEventAddHandle(fd, vir_events, cb_fd_event, fdinfo, free_fdinfo); + if (fdinfo->watch < 0) + return fdinfo->watch; + return 0; +} + +static int evhook_fd_modify(void *priv, int fd, void **hndp, short events) +{ + struct libxl_osevent_hook_fdinfo *fdinfo = *hndp; + (void)fd; (void)priv; + int vir_events = VIR_EVENT_HANDLE_ERROR; + if (events & POLLIN) + vir_events |= VIR_EVENT_HANDLE_READABLE; + if (events & POLLOUT) + vir_events |= VIR_EVENT_HANDLE_WRITABLE; + virEventUpdateHandle(fdinfo->watch, vir_events); + return 0; +} + +static void evhook_fd_deregister(void *priv, int fd, void *hnd) +{ + struct libxl_osevent_hook_fdinfo *fdinfo = hnd; + (void)priv; (void)fd; + virEventRemoveHandle(fdinfo->watch); +} + +struct libxl_osevent_hook_timerinfo { + libxlDomainObjPrivatePtr priv; + void *xl_priv; + int id; +}; + + +static void cb_timer(int timer, void *timer_v) +{ + struct libxl_osevent_hook_timerinfo *timer_info = timer_v; + (void)timer; + libxl_osevent_occurred_timeout(timer_info->priv->ctx, timer_info->xl_priv); +} + +static void timer_info_free(void* obj) +{ + free(obj); +} + +static int evhook_timeout_register(void *priv, void **hndp, struct timeval abs_t, void *for_libxl) +{ + struct timeval now; + struct libxl_osevent_hook_timerinfo *timer_info; + int timeout, timer_id; + timer_info = malloc(sizeof(*timer_info)); + gettimeofday(&now, NULL); + timeout = (abs_t.tv_usec - now.tv_usec) / 1000; + timeout += (abs_t.tv_sec - now.tv_sec) * 1000; + timer_id = virEventAddTimeout(timeout, cb_timer, timer_info, timer_info_free); + if (timer_id < 0) + return timer_id; + timer_info->priv = priv; + timer_info->xl_priv = for_libxl; + timer_info->id = timer_id; + *hndp = timer_info; + return 0; +} + +static int evhook_timeout_modify(void *priv, void **hndp, struct timeval abs_t) +{ + struct timeval now; + int timeout; + struct libxl_osevent_hook_timerinfo *timer_info = *hndp; + (void)priv; + gettimeofday(&now, NULL); + timeout = (abs_t.tv_usec - now.tv_usec) / 1000; + timeout += (abs_t.tv_sec - now.tv_sec) * 1000; + virEventUpdateTimeout(timer_info->id, timeout); + return 0; +} + +static void evhook_timeout_deregister(void *priv, void *hnd) { + struct libxl_osevent_hook_timerinfo *timer_info = hnd; + virEventRemoveTimeout(timer_info->id); + (void)priv; +} + +static const libxl_osevent_hooks event_callbacks = { + .fd_register = evhook_fd_register, + .fd_modify = evhook_fd_modify, + .fd_deregister = evhook_fd_deregister, + .timeout_register = evhook_timeout_register, + .timeout_modify = evhook_timeout_modify, + .timeout_deregister = evhook_timeout_deregister, +}; + static void * libxlDomainObjPrivateAlloc(void) { @@ -87,9 +220,9 @@ libxlDomainObjPrivateAlloc(void) if (VIR_ALLOC(priv) < 0) return NULL; - libxl_ctx_init(&priv->ctx, LIBXL_VERSION, libxl_driver->logger); - priv->waiterFD = -1; - priv->eventHdl = -1; + libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, libxl_driver->logger); + priv->deathW = NULL; + libxl_osevent_register_hooks(priv->ctx, &event_callbacks, priv); return priv; } @@ -99,16 +232,12 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data; - if (priv->eventHdl >= 0) - virEventRemoveHandle(priv->eventHdl); - - if (priv->dWaiter) { - libxl_stop_waiting(&priv->ctx, priv->dWaiter); - libxl_free_waiter(priv->dWaiter); - VIR_FREE(priv->dWaiter); + if (priv->deathW) { + libxl_evdisable_domain_death(priv->ctx, priv->deathW); + VIR_FREE(priv->deathW); } - libxl_ctx_free(&priv->ctx); + libxl_ctx_free(priv->ctx); VIR_FREE(priv); } @@ -120,17 +249,6 @@ libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) virDomainEventStateQueue(driver->domainEventState, event); } -/* - * Remove reference to domain object. - */ -static void -libxlDomainObjUnref(void *data) -{ - virDomainObjPtr vm = data; - - ignore_value(virDomainObjUnref(vm)); -} - static void libxlAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -161,13 +279,13 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) const libxl_version_info* ver_info; struct utsname utsname; - if (libxl_get_physinfo(&driver->ctx, &phy_info)) { + if (libxl_get_physinfo(driver->ctx, &phy_info)) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxl_get_physinfo_info failed")); return -1; } - if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxl_get_version_info failed")); return -1; @@ -291,15 +409,9 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, char *file; int i; - if (priv->eventHdl >= 0) { - virEventRemoveHandle(priv->eventHdl); - priv->eventHdl = -1; - } - - if (priv->dWaiter) { - libxl_stop_waiting(&priv->ctx, priv->dWaiter); - libxl_free_waiter(priv->dWaiter); - VIR_FREE(priv->dWaiter); + if (priv->deathW) { + libxl_evdisable_domain_death(priv->ctx, priv->deathW); + priv->deathW = NULL; } if (vm->persistent) { @@ -350,12 +462,11 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, static int libxlVmReap(libxlDriverPrivatePtr driver, virDomainObjPtr vm, - int force, virDomainShutoffReason reason) { libxlDomainObjPrivatePtr priv = vm->privateData; - if (libxl_domain_destroy(&priv->ctx, vm->def->id, force) < 0) { + if (libxl_domain_destroy(priv->ctx, vm->def->id) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Unable to cleanup domain %d"), vm->def->id); return -1; @@ -368,56 +479,26 @@ libxlVmReap(libxlDriverPrivatePtr driver, /* * Handle previously registered event notification from libxenlight */ -static void libxlEventHandler(int watch, - int fd, - int events, - void *data) +static void libxlEventHandler(void *data, const libxl_event *event) { libxlDriverPrivatePtr driver = libxl_driver; virDomainObjPtr vm = data; - libxlDomainObjPrivatePtr priv; virDomainEventPtr dom_event = NULL; - libxl_event event; - libxl_dominfo info; libxlDriverLock(driver); virDomainObjLock(vm); libxlDriverUnlock(driver); - priv = vm->privateData; - - memset(&event, 0, sizeof(event)); - memset(&info, 0, sizeof(info)); - - if (priv->waiterFD != fd || priv->eventHdl != watch) { - virEventRemoveHandle(watch); - priv->eventHdl = -1; - goto cleanup; - } - - if (!(events & VIR_EVENT_HANDLE_READABLE)) - goto cleanup; - - if (libxl_get_event(&priv->ctx, &event)) - goto cleanup; - - if (event.type == LIBXL_EVENT_DOMAIN_DEATH) { + if (event->type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) { virDomainShutoffReason reason; - /* libxl_event_get_domain_death_info returns 1 if death - * event was for this domid */ - if (libxl_event_get_domain_death_info(&priv->ctx, - vm->def->id, - &event, - &info) != 1) + if (event->domid != vm->def->id) goto cleanup; - virEventRemoveHandle(watch); - priv->eventHdl = -1; - switch (info.shutdown_reason) { - case SHUTDOWN_poweroff: - case SHUTDOWN_crash: - if (info.shutdown_reason == SHUTDOWN_crash) { + switch (event->u.domain_shutdown.shutdown_reason) { + case LIBXL_SHUTDOWN_REASON_POWEROFF: + case LIBXL_SHUTDOWN_REASON_CRASH: + if (event->u.domain_shutdown.shutdown_reason == LIBXL_SHUTDOWN_REASON_CRASH) { dom_event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); @@ -425,18 +506,18 @@ static void libxlEventHandler(int watch, } else { reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; } - libxlVmReap(driver, vm, 0, reason); + libxlVmReap(driver, vm, reason); if (!vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } break; - case SHUTDOWN_reboot: - libxlVmReap(driver, vm, 0, VIR_DOMAIN_SHUTOFF_SHUTDOWN); + case LIBXL_SHUTDOWN_REASON_REBOOT: + libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); libxlVmStart(driver, vm, 0, -1); break; default: - VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason); + VIR_INFO("Unhandled shutdown_reason %d", event->u.domain_shutdown.shutdown_reason); break; } } @@ -449,9 +530,14 @@ cleanup: libxlDomainEventQueue(driver, dom_event); libxlDriverUnlock(driver); } - libxl_free_event(&event); } +static const struct libxl_event_hooks ev_hooks = { + .event_occurs_mask = LIBXL_EVENTMASK_ALL, + .event_occurs = libxlEventHandler, + .disaster = NULL, +}; + /* * Register domain events with libxenlight and insert event handles * in libvirt''s event loop. @@ -460,40 +546,18 @@ static int libxlCreateDomEvents(virDomainObjPtr vm) { libxlDomainObjPrivatePtr priv = vm->privateData; - int fd; - - if (VIR_ALLOC(priv->dWaiter) < 0) { - virReportOOMError(); - return -1; - } - if (libxl_wait_for_domain_death(&priv->ctx, vm->def->id, priv->dWaiter)) - goto error; + libxl_event_register_callbacks(priv->ctx, &ev_hooks, vm); - libxl_get_wait_fd(&priv->ctx, &fd); - if (fd < 0) + if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) goto error; - priv->waiterFD = fd; - /* Add a reference to the domain object while it is injected in - * the event loop. - */ - virDomainObjRef(vm); - if ((priv->eventHdl = virEventAddHandle( - fd, - VIR_EVENT_HANDLE_READABLE | VIR_EVENT_HANDLE_ERROR, - libxlEventHandler, - vm, libxlDomainObjUnref)) < 0) { - ignore_value(virDomainObjUnref(vm)); - goto error; - } - return 0; error: - libxl_free_waiter(priv->dWaiter); - VIR_FREE(priv->dWaiter); - priv->eventHdl = -1; + if (priv->deathW) + libxl_evdisable_domain_death(priv->ctx, priv->deathW); + free(priv->deathW); return -1; } @@ -534,7 +598,7 @@ libxlDomainSetVcpuAffinites(libxlDriverPrivatePtr driver, virDomainObjPtr vm) map.size = cpumaplen; map.map = cpumap; - if (libxl_set_vcpuaffinity(&priv->ctx, def->id, vcpu, &map) != 0) { + if (libxl_set_vcpuaffinity(priv->ctx, def->id, vcpu, &map) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to pin vcpu ''%d'' with libxenlight"), vcpu); goto cleanup; @@ -560,11 +624,10 @@ libxlFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) int tries = 3; int wait_secs = 10; - if ((ret = libxl_domain_need_memory(&priv->ctx, &d_config->b_info, - &d_config->dm_info, + if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem)) >= 0) { for (i = 0; i < tries; ++i) { - if ((ret = libxl_get_free_memory(&priv->ctx, &free_mem)) < 0) + if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0) break; if (free_mem >= needed_mem) { @@ -572,17 +635,17 @@ libxlFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) break; } - if ((ret = libxl_set_memory_target(&priv->ctx, 0, + if ((ret = libxl_set_memory_target(priv->ctx, 0, free_mem - needed_mem, /* relative */ 1, 0)) < 0) break; - ret = libxl_wait_for_free_memory(&priv->ctx, 0, needed_mem, + ret = libxl_wait_for_free_memory(priv->ctx, 0, needed_mem, wait_secs); if (ret == 0 || ret != ERROR_NOMEM) break; - if ((ret = libxl_wait_for_memory_target(&priv->ctx, 0, 1)) < 0) + if ((ret = libxl_wait_for_memory_target(priv->ctx, 0, 1)) < 0) break; } } @@ -651,7 +714,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, VIR_FREE(managed_save_path); } - memset(&d_config, 0, sizeof(d_config)); + libxl_domain_config_init(&d_config); if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0 ) return -1; @@ -664,10 +727,10 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } if (restore_fd < 0) - ret = libxl_domain_create_new(&priv->ctx, &d_config, + ret = libxl_domain_create_new(priv->ctx, &d_config, NULL, &child_console_pid, &domid); else - ret = libxl_domain_create_restore(&priv->ctx, &d_config, NULL, + ret = libxl_domain_create_restore(priv->ctx, &d_config, NULL, &child_console_pid, &domid, restore_fd); @@ -687,7 +750,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) goto error; - if (libxl_userdata_store(&priv->ctx, domid, "libvirt-xml", + if (libxl_userdata_store(priv->ctx, domid, "libvirt-xml", (uint8_t *)dom_xml, strlen(dom_xml) + 1)) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to store userdata")); @@ -701,7 +764,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto error; if (!start_paused) { - libxl_domain_unpause(&priv->ctx, domid); + libxl_domain_unpause(priv->ctx, domid); virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); } else { virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); @@ -717,18 +780,18 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, VIR_DOMAIN_EVENT_STARTED_RESTORED); libxlDomainEventQueue(driver, event); - libxl_domain_config_destroy(&d_config); + libxl_domain_config_dispose(&d_config); VIR_FREE(dom_xml); VIR_FORCE_CLOSE(managed_save_fd); return 0; error: if (domid > 0) { - libxl_domain_destroy(&priv->ctx, domid, 0); + libxl_domain_destroy(priv->ctx, domid); vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); } - libxl_domain_config_destroy(&d_config); + libxl_domain_config_dispose(&d_config); VIR_FREE(dom_xml); VIR_FREE(managed_save_path); virDomainDefFree(def); @@ -756,7 +819,7 @@ libxlReconnectDomain(void *payload, virDomainObjLock(vm); /* Does domain still exist? */ - rc = libxl_domain_info(&driver->ctx, &d_info, vm->def->id); + rc = libxl_domain_info(driver->ctx, &d_info, vm->def->id); if (rc == ERROR_INVAL) { goto out; } else if (rc != 0) { @@ -766,7 +829,7 @@ libxlReconnectDomain(void *payload, } /* Is this a domain that was under libvirt control? */ - if (libxl_userdata_retrieve(&driver->ctx, vm->def->id, + if (libxl_userdata_retrieve(driver->ctx, vm->def->id, "libvirt-xml", &data, &len)) { VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", vm->def->id); goto out; @@ -804,7 +867,7 @@ libxlShutdown(void) libxlDriverLock(libxl_driver); virCapabilitiesFree(libxl_driver->caps); virDomainObjListDeinit(&libxl_driver->domains); - libxl_ctx_free(&libxl_driver->ctx); + libxl_ctx_free(libxl_driver->ctx); xtl_logger_destroy(libxl_driver->logger); if (libxl_driver->logger_file) VIR_FORCE_FCLOSE(libxl_driver->logger_file); @@ -937,14 +1000,14 @@ libxlStartup(int privileged) { goto fail; } - if (libxl_ctx_init(&libxl_driver->ctx, - LIBXL_VERSION, + if (libxl_ctx_alloc(&libxl_driver->ctx, + LIBXL_VERSION, 0, libxl_driver->logger)) { VIR_INFO("cannot initialize libxenlight context, probably not running in a Xen Dom0, disabling driver"); goto fail; } - if ((ver_info = libxl_get_version_info(&libxl_driver->ctx)) == NULL) { + if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) { VIR_INFO("cannot version information from libxenlight, disabling driver"); goto fail; } @@ -952,7 +1015,7 @@ libxlStartup(int privileged) { (ver_info->xen_version_minor * 1000); if ((libxl_driver->caps - libxlMakeCapabilities(&libxl_driver->ctx)) == NULL) { + libxlMakeCapabilities(libxl_driver->ctx)) == NULL) { VIR_ERROR(_("cannot create capabilities for libxenlight")); goto error; } @@ -1112,7 +1175,7 @@ libxlGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) int ret; libxlDriverPrivatePtr driver = conn->privateData; - ret = libxl_get_max_cpus(&driver->ctx); + ret = libxl_get_max_cpus(driver->ctx); /* libxl_get_max_cpus() will return 0 if there were any failures, e.g. xc_physinfo() failing */ if (ret == 0) @@ -1317,7 +1380,7 @@ libxlDomainSuspend(virDomainPtr dom) priv = vm->privateData; if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - if (libxl_domain_pause(&priv->ctx, dom->id) != 0) { + if (libxl_domain_pause(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to suspend domain ''%d'' with libxenlight"), dom->id); @@ -1376,7 +1439,7 @@ libxlDomainResume(virDomainPtr dom) priv = vm->privateData; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - if (libxl_domain_unpause(&priv->ctx, dom->id) != 0) { + if (libxl_domain_unpause(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to resume domain ''%d'' with libxenlight"), dom->id); @@ -1433,7 +1496,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) } priv = vm->privateData; - if (libxl_domain_shutdown(&priv->ctx, dom->id, LIBXL_DOM_REQ_POWEROFF) != 0) { + if (libxl_domain_shutdown(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to shutdown domain ''%d'' with libxenlight"), dom->id); @@ -1486,7 +1549,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } priv = vm->privateData; - if (libxl_domain_shutdown(&priv->ctx, dom->id, LIBXL_DOM_REQ_REBOOT) != 0) { + if (libxl_domain_reboot(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to reboot domain ''%d'' with libxenlight"), dom->id); @@ -1531,7 +1594,7 @@ libxlDomainDestroyFlags(virDomainPtr dom, event = virDomainEventNewFromObj(vm,VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_DESTROYED) != 0) { + if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain ''%d''"), dom->id); goto cleanup; @@ -1669,7 +1732,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_MEM_LIVE) { priv = vm->privateData; - if (libxl_domain_setmaxmem(&priv->ctx, dom->id, newmem) < 0) { + if (libxl_domain_setmaxmem(priv->ctx, dom->id, newmem) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to set maximum memory for domain ''%d''" " with libxenlight"), dom->id); @@ -1698,7 +1761,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_MEM_LIVE) { priv = vm->privateData; - if (libxl_set_memory_target(&priv->ctx, dom->id, newmem, 0, + if (libxl_set_memory_target(priv->ctx, dom->id, newmem, 0, /* force */ 1) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to set memory for domain ''%d''" @@ -1758,7 +1821,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) info->memory = vm->def->mem.cur_balloon; info->maxMem = vm->def->mem.max_balloon; } else { - if (libxl_domain_info(&driver->ctx, &d_info, dom->id) != 0) { + if (libxl_domain_info(driver->ctx, &d_info, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxl_domain_info failed for domain ''%d''"), dom->id); goto cleanup; @@ -1858,7 +1921,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup; } - if (libxl_domain_suspend(&priv->ctx, NULL, vm->def->id, fd) != 0) { + if (libxl_domain_suspend(priv->ctx, NULL, vm->def->id, fd) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to save domain ''%d'' with libxenlight"), vm->def->id); @@ -1868,7 +1931,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); - if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { + if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain ''%d''"), vm->def->id); goto cleanup; @@ -2023,7 +2086,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) if (!(flags & VIR_DUMP_LIVE) && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { - if (libxl_domain_pause(&priv->ctx, dom->id) != 0) { + if (libxl_domain_pause(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Before dumping core, failed to suspend domain ''%d''" " with libxenlight"), @@ -2034,7 +2097,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) paused = true; } - if (libxl_domain_core_dump(&priv->ctx, dom->id, to) != 0) { + if (libxl_domain_core_dump(priv->ctx, dom->id, to) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to dump core of domain ''%d'' with libxenlight"), dom->id); @@ -2043,7 +2106,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) libxlDriverLock(driver); if (flags & VIR_DUMP_CRASH) { - if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) { + if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to destroy domain ''%d''"), dom->id); goto cleanup_unlock; @@ -2064,7 +2127,7 @@ cleanup_unlock: libxlDriverUnlock(driver); cleanup_unpause: if (virDomainObjIsActive(vm) && paused) { - if (libxl_domain_unpause(&priv->ctx, dom->id) != 0) { + if (libxl_domain_unpause(priv->ctx, dom->id) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("After dumping core, failed to resume domain ''%d'' with" " libxenlight"), dom->id); @@ -2301,7 +2364,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE: - if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) { + if (libxl_set_vcpuonline(priv->ctx, dom->id, &map) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain ''%d''" " with libxenlight"), dom->id); @@ -2310,7 +2373,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: - if (libxl_set_vcpuonline(&priv->ctx, dom->id, &map) != 0) { + if (libxl_set_vcpuonline(priv->ctx, dom->id, &map) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to set vcpus for domain ''%d''" " with libxenlight"), dom->id); @@ -2427,7 +2490,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, map.size = maplen; map.map = cpumap; - if (libxl_set_vcpuaffinity(&priv->ctx, dom->id, vcpu, &map) != 0) { + if (libxl_set_vcpuaffinity(priv->ctx, dom->id, vcpu, &map) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to pin vcpu ''%d'' with libxenlight"), vcpu); goto cleanup; @@ -2479,7 +2542,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, } priv = vm->privateData; - if ((vcpuinfo = libxl_list_vcpu(&priv->ctx, dom->id, &maxcpu, + if ((vcpuinfo = libxl_list_vcpu(priv->ctx, dom->id, &maxcpu, &hostcpus)) == NULL) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to list vcpus for domain ''%d'' with libxenlight"), @@ -2506,7 +2569,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, MIN(maplen, vcpuinfo[i].cpumap.size)); } - libxl_vcpuinfo_destroy(&vcpuinfo[i]); + libxl_vcpuinfo_dispose(&vcpuinfo[i]); } VIR_FREE(vcpuinfo); @@ -2564,7 +2627,7 @@ libxlDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat, goto cleanup; } - if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) { VIR_ERROR(_("cannot get version information from libxenlight")); goto cleanup; } @@ -2607,7 +2670,7 @@ libxlDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, goto cleanup; } - if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) { VIR_ERROR(_("cannot get version information from libxenlight")); goto cleanup; } @@ -2870,7 +2933,7 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(vm->def, disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_cdrom_insert(&priv->ctx, vm->def->id, &x_disk)) < 0) { + if ((ret = libxl_cdrom_insert(priv->ctx, vm->def->id, &x_disk)) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to change media for disk ''%s''"), disk->dst); @@ -2925,7 +2988,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(vm->def, l_disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_device_disk_add(&priv->ctx, vm->def->id, + if ((ret = libxl_device_disk_add(priv->ctx, vm->def->id, &x_disk)) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to attach disk ''%s''"), @@ -2959,7 +3022,6 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, virDomainDiskDefPtr l_disk = NULL; libxl_device_disk x_disk; int i; - int wait_secs = 2; int ret = -1; switch (dev->data.disk->device) { @@ -2979,8 +3041,7 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, if (libxlMakeDisk(vm->def, l_disk, &x_disk) < 0) goto cleanup; - if ((ret = libxl_device_disk_del(&priv->ctx, &x_disk, - wait_secs)) < 0) { + if ((ret = libxl_device_disk_remove(priv->ctx, vm->def->id, &x_disk, NULL)) < 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to detach disk ''%s''"), l_disk->dst); @@ -3355,12 +3416,12 @@ libxlNodeGetFreeMemory(virConnectPtr conn) const libxl_version_info* ver_info; libxlDriverPrivatePtr driver = conn->privateData; - if (libxl_get_physinfo(&driver->ctx, &phy_info)) { + if (libxl_get_physinfo(driver->ctx, &phy_info)) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxl_get_physinfo_info failed")); return 0; } - if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) { + if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) { libxlError(VIR_ERR_INTERNAL_ERROR, _("libxl_get_version_info failed")); return 0; } @@ -3506,7 +3567,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; char * ret = NULL; - int sched_id; + libxl_scheduler sched_id; libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3523,31 +3584,29 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) } priv = vm->privateData; - if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get scheduler id for domain ''%d''" - " with libxenlight"), dom->id); - goto cleanup; - } + sched_id = libxl_get_scheduler(priv->ctx); if (nparams) *nparams = 0; switch(sched_id) { - case XEN_SCHEDULER_SEDF: + case LIBXL_SCHEDULER_SEDF: ret = strdup("sedf"); break; - case XEN_SCHEDULER_CREDIT: + case LIBXL_SCHEDULER_CREDIT: ret = strdup("credit"); if (nparams) *nparams = XEN_SCHED_CREDIT_NPARAM; break; - case XEN_SCHEDULER_CREDIT2: + case LIBXL_SCHEDULER_CREDIT2: ret = strdup("credit2"); break; - case XEN_SCHEDULER_ARINC653: + case LIBXL_SCHEDULER_ARINC653: ret = strdup("arinc653"); break; default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Failed to get scheduler id for domain ''%d''" + " with libxenlight"), dom->id); goto cleanup; } @@ -3566,11 +3625,16 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, int *nparams, unsigned int flags) { + (void)dom; + (void)params; + (void)nparams; + (void)flags; + libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; - libxl_sched_credit sc_info; - int sched_id; + libxl_sched_credit_domain sc_info; + libxl_scheduler sched_id; int ret = -1; virCheckFlags(0, -1); @@ -3591,20 +3655,15 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, priv = vm->privateData; - if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get scheduler id for domain ''%d''" - " with libxenlight"), dom->id); - goto cleanup; - } + sched_id = libxl_get_scheduler(priv->ctx); - if (sched_id != XEN_SCHEDULER_CREDIT) { + if (sched_id != LIBXL_SCHEDULER_CREDIT) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Only ''credit'' scheduler is supported")); goto cleanup; } - if (libxl_sched_credit_domain_get(&priv->ctx, dom->id, &sc_info) != 0) { + if (libxl_sched_credit_domain_get(priv->ctx, dom->id, &sc_info) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to get scheduler parameters for domain ''%d''" " with libxenlight"), dom->id); @@ -3644,10 +3703,14 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, int nparams, unsigned int flags) { + (void)dom; + (void)params; + (void)nparams; + (void)flags; libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDomainObjPrivatePtr priv; virDomainObjPtr vm; - libxl_sched_credit sc_info; + libxl_sched_credit_domain sc_info; int sched_id; int i; int ret = -1; @@ -3677,20 +3740,15 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, priv = vm->privateData; - if ((sched_id = libxl_get_sched_id(&priv->ctx)) < 0) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get scheduler id for domain ''%d''" - " with libxenlight"), dom->id); - goto cleanup; - } + sched_id = libxl_get_scheduler(priv->ctx); - if (sched_id != XEN_SCHEDULER_CREDIT) { + if (sched_id != LIBXL_SCHEDULER_CREDIT) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Only ''credit'' scheduler is supported")); goto cleanup; } - if (libxl_sched_credit_domain_get(&priv->ctx, dom->id, &sc_info) != 0) { + if (libxl_sched_credit_domain_get(priv->ctx, dom->id, &sc_info) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to get scheduler parameters for domain ''%d''" " with libxenlight"), dom->id); @@ -3707,7 +3765,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, } } - if (libxl_sched_credit_domain_set(&priv->ctx, dom->id, &sc_info) != 0) { + if (libxl_sched_credit_domain_set(priv->ctx, dom->id, &sc_info) != 0) { libxlError(VIR_ERR_INTERNAL_ERROR, _("Failed to set scheduler parameters for domain ''%d''" " with libxenlight"), dom->id); -- 1.7.7.6
On Wed, 2012-05-09 at 18:13 +0100, Daniel De Graaf wrote:> This patch changes libxl to use the interface in Xen 4.2. It is provided > as an example, not intended to go in to libvirt as-is since it removes > all support for libxl from Xen 4.1. It also still has some cruft (extra > void casts on parameters) and the device model info population is not > written. It has been tested with simple domain create/destroy. > --- > src/libxl/libxl_conf.c | 134 +++++++++------ > src/libxl/libxl_conf.h | 8 +- > src/libxl/libxl_driver.c | 430 ++++++++++++++++++++++++++-------------------- > 3 files changed, 326 insertions(+), 246 deletions(-)Thanks Daniel, interesting to see. It doesn''t seem as invasive as I expected given the number of changes to libxl''s interfaces between 4.1 and 4.2. I suppose it is worth mentioning in this context that we are intending to maintain the 4.2 libxl API in a stable manner going forward, as described near the top of: http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/tip/tools/libxl/libxl.h Since libvirt is one of the main reasons we are doing this it would be useful to check that this actually meets the needs from that side. That doesn''t really help with support 4.1 and 4.2+. A large portion of the below looks like it would be reasonably easy to abstract away with a header full of compat defines for naming changes etc, other bits don''t look so simple to deal with.> @@ -403,9 +414,9 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) > return -1; > } > > - libxl_init_build_info(b_info, &d_config->c_info); > + libxl_domain_build_info_init(b_info); > > - b_info->hvm = hvm; > + b_info->type = hvm ? LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PV;I''m not sure what version of "4.2" you are currently using but this should become libxl_domain_build_info_init_type(b_info, hvm ? LIBXL...) at some point.> b_info->max_vcpus = def->maxvcpus; > if (def->vcpus == 32) > b_info->cur_vcpus = (uint32_t) -1;[...]> @@ -454,7 +466,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) > } > } > if (def->os.bootloaderArgs) { > - if ((b_info->u.pv.bootloader_args = strdup(def->os.bootloaderArgs)) == NULL) { > + // XXX may need to split these arguments on a delimiterI think you do. We could consider moving split_string_into_string_list from xl into the xlu(tility) library if libvirt doesn''t have an existing helper for that, or you can just copy it (I''m the sole author of that function, I''d be happy to ack any reasonable license for use in libvirt, some variant of (L)GPL I presume?) [...]> +static const libxl_osevent_hooks event_callbacks = { > + .fd_register = evhook_fd_register, > + .fd_modify = evhook_fd_modify, > + .fd_deregister = evhook_fd_deregister, > + .timeout_register = evhook_timeout_register, > + .timeout_modify = evhook_timeout_modify, > + .timeout_deregister = evhook_timeout_deregister, > +};Glad to see that this interface seems to fit in well with libvirt''s infrastructure -- that was the point of it after all ;-)> +static const struct libxl_event_hooks ev_hooks = { > + .event_occurs_mask = LIBXL_EVENTMASK_ALL, > + .event_occurs = libxlEventHandler, > + .disaster = NULL, > +};Same for this interface. Ian.
Daniel P. Berrange
2012-May-10 10:10 UTC
Re: [libvirt] [PATCH-RFC] Change libxl to use Xen 4.2 interface
On Thu, May 10, 2012 at 10:56:06AM +0100, Ian Campbell wrote:> On Wed, 2012-05-09 at 18:13 +0100, Daniel De Graaf wrote: > > This patch changes libxl to use the interface in Xen 4.2. It is provided > > as an example, not intended to go in to libvirt as-is since it removes > > all support for libxl from Xen 4.1. It also still has some cruft (extra > > void casts on parameters) and the device model info population is not > > written. It has been tested with simple domain create/destroy. > > --- > > src/libxl/libxl_conf.c | 134 +++++++++------ > > src/libxl/libxl_conf.h | 8 +- > > src/libxl/libxl_driver.c | 430 ++++++++++++++++++++++++++-------------------- > > 3 files changed, 326 insertions(+), 246 deletions(-) > > Thanks Daniel, interesting to see. It doesn''t seem as invasive as I > expected given the number of changes to libxl''s interfaces between 4.1 > and 4.2. > > I suppose it is worth mentioning in this context that we are intending > to maintain the 4.2 libxl API in a stable manner going forward, as > described near the top of: > http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/tip/tools/libxl/libxl.h > > Since libvirt is one of the main reasons we are doing this it would be > useful to check that this actually meets the needs from that side.Having a long term stable API for libxl certainly gets our vote. WRT the use of LIBXL_API_VERSION to control API version. Libvirt generally aims to support all possible API versions, so I expect we would not want to define LIBXL_API_VERSION. Instead the libvirt code would #ifdef various bits according to the libxl version it detects.> That doesn''t really help with support 4.1 and 4.2+. A large portion of > the below looks like it would be reasonably easy to abstract away with a > header full of compat defines for naming changes etc, other bits don''t > look so simple to deal with.Personally I would prefer to keep support for all 4.x versions, but I''ll defer to Suse developers to decide upon this since they are the primary maintainers & users of the libxl driver. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Ian Jackson
2012-May-10 11:20 UTC
Re: [PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro
Daniel De Graaf writes ("Re: [Xen-devel] [PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro"):> I like that method (or this slight tweak to it): > > -----------8<------------------------- > > The OSEVENT_HOOK_INTERN macro incorrectly returned the value of the > expression CTX->osevent_in_hook-- (usually 1) instead of the value of > the function call it made. Fix the macro to return the proper value.Excellent, thanks. Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Jim Fehlig
2012-May-10 22:41 UTC
Re: [libvirt] [PATCH-RFC] Change libxl to use Xen 4.2 interface
Daniel P. Berrange wrote:> On Thu, May 10, 2012 at 10:56:06AM +0100, Ian Campbell wrote: > >> On Wed, 2012-05-09 at 18:13 +0100, Daniel De Graaf wrote: >> >>> This patch changes libxl to use the interface in Xen 4.2. It is provided >>> as an example, not intended to go in to libvirt as-is since it removes >>> all support for libxl from Xen 4.1. It also still has some cruft (extra >>> void casts on parameters) and the device model info population is not >>> written. It has been tested with simple domain create/destroy. >>> --- >>> src/libxl/libxl_conf.c | 134 +++++++++------ >>> src/libxl/libxl_conf.h | 8 +- >>> src/libxl/libxl_driver.c | 430 ++++++++++++++++++++++++++-------------------- >>> 3 files changed, 326 insertions(+), 246 deletions(-) >>> >> Thanks Daniel, interesting to see. It doesn''t seem as invasive as I >> expected given the number of changes to libxl''s interfaces between 4.1 >> and 4.2. >> >> I suppose it is worth mentioning in this context that we are intending >> to maintain the 4.2 libxl API in a stable manner going forward, as >> described near the top of: >> http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/tip/tools/libxl/libxl.h >> >> Since libvirt is one of the main reasons we are doing this it would be >> useful to check that this actually meets the needs from that side. >> > > Having a long term stable API for libxl certainly gets our vote. > > WRT the use of LIBXL_API_VERSION to control API version. Libvirt generally > aims to support all possible API versions, so I expect we would not want > to define LIBXL_API_VERSION. Instead the libvirt code would #ifdef various > bits according to the libxl version it detects. > > >> That doesn''t really help with support 4.1 and 4.2+. A large portion of >> the below looks like it would be reasonably easy to abstract away with a >> header full of compat defines for naming changes etc, other bits don''t >> look so simple to deal with. >> > > Personally I would prefer to keep support for all 4.x versions, but I''ll > defer to Suse developers to decide upon this since they are the primary > maintainers & users of the libxl driver. >Yes, I too would like to keep support for all 4.x versions. As pointed out by you and Ian, it can be detected at build time vs the runtime probing that the legacy xen libvirt driver does. Bamvor has just started looking into this, so hopefully he can help with the effort. Regards, Jim