|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/5] libxl: call hotplug scripts from libxl for vbd
El 16/04/2012, a las 17:44, Ian Campbell escribió:
> On Mon, 2012-04-16 at 16:06 +0100, Roger Pau Monne wrote:
>> This is a rather big change, that adds the necessary machinery to perform
>> hotplug script calling from libxl for Linux. This patch launches the
>> necessary
>> scripts to attach and detach PHY and TAP backend types (Qemu doesn't use
>> hotplug
>> scripts).
>>
>> Here's a list of the major changes introduced:
>>
>> * libxl_device_disk_add makes use of the new event library and takes the
>> optional parameter libxl_asyncop_how, to choose how the operation has to be
>> issued (sync or async).
>>
>> * libxl_device_disk_add waits for backend to switch to state 2
>> (XenbusInitWait)
>> and then launches the hotplug script.
>>
>> * libxl__devices_destroy no longer calls libxl__device_destroy, and instead
>> calls libxl__initiate_device_remove, so we can disconnect the device and
>> execute the necessary hotplug scripts instead of just deleting the backend
>> entries. So libxl__devices_destroy now uses the event library, and so does
>> libxl_domain_destroy.
>>
>> * Since libxl__devices_destroy calls multiple times
>> libxl__initiate_device_remove, this function now returns a different value
>> regarding the actions taken (if an event was added or not). The user has to
>> call libxl__ao_complete after using this function if necessary.
>>
>> * The internal API for hotplug scripts has been added, which consists of one
>> function; libxl__device_hotplug that takes the device and the action to
>> execute.
>>
>> * Linux hotplug scripts are called by setting the necessary env variables to
>> emulate udev rules, so there's no need to change them (although a rework to
>> pass this as parameters insted of env variables would be suitable, so both
>> NetBSD and Linux hotplug scripts take the same parameters).
>>
>> * Added a check in xen-hotplug-common.sh, so scripts are only executed from
>> udev when using xend. xl adds an execute_udev=n to xenstore device backend
>> and passes XL_CALL=y to the env of the script call, and udev calls check
>> that
>> execute_udev is not set and XL_CALL is empty also.
>>
>> I've also modified the python binding for pyxl_domain_destroy, that now take
>> the
>> ao_how parameter, but I'm not really sure if it's done correctly, let's just
>> say
>> that it compiles.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
>> ---
>> tools/hotplug/Linux/xen-hotplug-common.sh | 6 ++
>> tools/libxl/Makefile | 3 +-
>> tools/libxl/libxl.c | 104 ++++++++++++++++++----
>> tools/libxl/libxl.h | 7 +-
>> tools/libxl/libxl_create.c | 4 +-
>> tools/libxl/libxl_device.c | 140
>> +++++++++++++++++++++++++++--
>> tools/libxl/libxl_dm.c | 4 +-
>> tools/libxl/libxl_hotplug.c | 67 ++++++++++++++
>> tools/libxl/libxl_internal.h | 43 +++++++++-
>> tools/libxl/libxl_linux.c | 117 ++++++++++++++++++++++++
>> tools/libxl/xl_cmdimpl.c | 14 ++--
>> tools/python/xen/lowlevel/xl/xl.c | 5 +-
>> 12 files changed, 474 insertions(+), 40 deletions(-)
>> create mode 100644 tools/libxl/libxl_hotplug.c
>>
>> diff --git a/tools/hotplug/Linux/xen-hotplug-common.sh
>> b/tools/hotplug/Linux/xen-hotplug-common.sh
>> index 8f6557d..dc3e867 100644
>> --- a/tools/hotplug/Linux/xen-hotplug-common.sh
>> +++ b/tools/hotplug/Linux/xen-hotplug-common.sh
>> @@ -15,6 +15,12 @@
>> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> #
>>
>> +# Hack to prevent the execution of hotplug scripts form udev if the domain
>
> from
>> +# has ben launched from libxl
>
> been
Done
>
>> +execute_udev=`xenstore-read $XENBUS_PATH/execute_udev 2>/dev/null`
>> +if [ -n "$execute_udev" ] && [ -z "$XL_CALL" ]; then
>> + exit 0
>> +fi
>
> So, am I right that in some future world where we have got rid of xend
> and made this stuff work without udev everywhere (e.g. including driver
> domains) there is a path to deprecating this snippet without requiring
> everyone to go through some sort of transition?
>
> Or are we stuck with this envvar forever? It's not a big deal but if we
> can invert it (e.g. by setting ENV{HOTPLUG_GATE}=${XENBUS}/evecute_udev
> in xen-backend.rules and ...if -n "${HOTPLUG_GATE}" && xenstore-read
> ${HOTPLUG_GATE}... etc) then that would be nicer?
I will change it to something like:
if [ -z "${HOTPLUG_GATE}" ] && `xenstore-read "$HOTPLUG_GATE" 2>/dev/null` && \
[ -z "$XL_CALL" ]; then
and then set HOTPLUG_GATE from udev rules. I will change the xenstore gate to
"disable_udev", which makes more sense, since we will be setting
"disable_udev=y" or nothing.
>> dir=$(dirname "$0")
>> . "$dir/hotplugpath.sh"
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 16ebef3..fb4fbf2 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1062,13 +1062,15 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx,
>> libxl_evgen_disk_eject *evg) {
>> GC_FREE;
>> }
>>
>> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
>> +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
>> + const libxl_asyncop_how *ao_how)
>> {
>> - GC_INIT(ctx);
>> + AO_CREATE(ctx, domid, ao_how);
>> char *dom_path;
>> char *vm_path;
>> char *pid;
>> int rc, dm_present;
>> + int rc_ao = 0;
>>
>> rc = libxl_domain_info(ctx, NULL, domid);
>> switch(rc) {
>> @@ -1110,7 +1112,8 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t
>> domid)
>>
>> libxl__qmp_cleanup(gc, domid);
>> }
>> - if (libxl__devices_destroy(gc, domid) < 0)
>> + rc_ao = libxl__devices_destroy(egc, ao, domid);
>> + if (rc_ao < 0)
>> LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
>> "libxl__devices_destroy failed for %d", domid);
>>
>> @@ -1133,9 +1136,10 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t
>> domid)
>> goto out;
>> }
>> rc = 0;
>> +
>> out:
>> - GC_FREE;
>> - return rc;
>> + if (rc_ao) return AO_ABORT(rc_ao);
>> + return AO_INPROGRESS;
>> }
>>
>> int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
>> libxl_console_type type)
>> @@ -1313,9 +1317,11 @@ static int libxl__device_from_disk(libxl__gc *gc,
>> uint32_t domid,
>> return 0;
>> }
>>
>> -int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk
>> *disk)
>> +int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
>> + libxl_device_disk *disk,
>> + const libxl_asyncop_how *ao_how)
>> {
>> - GC_INIT(ctx);
>> + AO_CREATE(ctx, domid, ao_how);
>> flexarray_t *front;
>> flexarray_t *back;
>> char *dev;
>> @@ -1330,7 +1336,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
>> domid, libxl_device_disk *dis
>> rc = ERROR_NOMEM;
>> goto out;
>> }
>> - back = flexarray_make(16, 1);
>> + back = flexarray_make(20, 1);
>> if (!back) {
>> rc = ERROR_NOMEM;
>> goto out_free;
>> @@ -1361,6 +1367,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
>> domid, libxl_device_disk *dis
>> flexarray_append(back, "params");
>> flexarray_append(back, dev);
>>
>> + flexarray_append(back, "script");
>> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
>> +
>> libxl_xen_script_dir_path(),
>> + "block"));
>> +
>> assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
>> break;
>> case LIBXL_DISK_BACKEND_TAP:
>> @@ -1374,6 +1385,11 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
>> domid, libxl_device_disk *dis
>> libxl__device_disk_string_of_format(disk->format),
>> disk->pdev_path));
>>
>> + flexarray_append(back, "script");
>> + flexarray_append(back, libxl__sprintf(gc, "%s/%s",
>> + libxl_xen_script_dir_path(),
>> + "blktap"));
>> +
>> /* now create a phy device to export the device to the guest */
>> goto do_backend_phy;
>> case LIBXL_DISK_BACKEND_QDISK:
>> @@ -1406,6 +1422,9 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
>> domid, libxl_device_disk *dis
>> flexarray_append(back, disk->readwrite ? "w" : "r");
>> flexarray_append(back, "device-type");
>> flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
>> + /* compatibility addon to keep udev rules */
>> + flexarray_append(back, "execute_udev");
>> + flexarray_append(back, "n");
>>
>> flexarray_append(front, "backend-id");
>> flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
>> @@ -1420,14 +1439,23 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
>> domid, libxl_device_disk *dis
>> libxl__xs_kvs_of_flexarray(gc, back,
>> back->count),
>> libxl__xs_kvs_of_flexarray(gc, front,
>> front->count));
>>
>> + if (disk->backend == LIBXL_DISK_BACKEND_PHY ||
>> + disk->backend == LIBXL_DISK_BACKEND_TAP) {
>> + rc = libxl__initiate_device_add(egc, ao, &device);
>> + if (rc) goto out_free;
>> + } else {
>> + /* no need to execute hotplug scripts */
>> + libxl__ao_complete(egc, ao, 0);
>> + }
>
> This would be better as a switch, since it would make it more obvious
> that the "else" is actually "case LIBXL_DISK_BACKEND_QDISK" etc.
>
Yes
> [...]
>> +
>> +/*
>> + * Return codes:
>> + * 1 Success and event(s) HAVE BEEN added
>> + * 0 Success and NO events where added
>
> were
>
> (I saw a few of these)
Will do a grep to see what I can find…
Thanks for the review.
>
>> + * < 0 Error
>> + *
>> + * Since this function can be called several times, and several events
>> + * can be added to the same context, the user has to call
>> + * libxl__ao_complete when necessary (if no events are added).
>> + */
>> int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
>> libxl__device *dev)
>> {
>> @@ -392,7 +461,6 @@ int libxl__initiate_device_remove(libxl__egc *egc,
>> libxl__ao *ao,
> [...]
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |