From: George Dunlap <george.dunlap@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xen.org
Cc: Ian Jackson <ian.jackson@citrix.com>,
Simon Cao <caobosimon@gmail.com>, Wei Liu <wei.liu2@citrix.com>,
Chunyan Liu <cyliu@suse.com>,
Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v10 3/5] libxl: add pvusb API
Date: Thu, 10 Dec 2015 12:08:43 +0000 [thread overview]
Message-ID: <56696B4B.7060801@citrix.com> (raw)
In-Reply-To: <1449749113-1243-4-git-send-email-george.dunlap@eu.citrix.com>
[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]
On 10/12/15 12:05, George Dunlap wrote:
> From: Chunyan Liu <cyliu@suse.com>
>
> Add pvusb APIs, including:
> - attach/detach (create/destroy) virtual usb controller.
> - attach/detach usb device
> - list usb controller and usb devices
> - some other helper functions
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> Signed-off-by: Simon Cao <caobosimon@gmail.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Attached is a diff of v9 -> v10 for convenience.
One remaining question I had regarding this patch...
> +static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid,
> + char ***intfs, int *num)
> +{
> + DIR *dir;
> + char *buf;
> + int rc;
> +
> + *intfs = NULL;
> + *num = 0;
> +
> + buf = GCSPRINTF("%s:", busid);
> +
> + dir = opendir(SYSFS_USB_DEV);
> + if (!dir) {
> + LOGE(ERROR, "opendir failed: '%s'", SYSFS_USB_DEV);
> + return ERROR_FAIL;
> + }
> +
> + size_t need = offsetof(struct dirent, d_name) +
> + pathconf(SYSFS_USB_DEV, _PC_NAME_MAX) + 1;
> + struct dirent *de_buf = libxl__zalloc(gc, need);
Is this thing with manually calculating the size of the structure really
necessary? Could we not just declare "struct dirent de_buf" on the stack?
If it is necessary, it would be better to have it inside a function or
macro called "alloc_dirent" or something like that.
-George
[-- Attachment #2: pvusb-p3-v9-v10.diff --]
[-- Type: text/x-patch, Size: 18449 bytes --]
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a479465..26cd5fa 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3203,7 +3203,7 @@ void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
aodev->dev = device;
aodev->callback = local_device_detach_cb;
aodev->force = 0;
- libxl__initiate_device_remove(egc, aodev);
+ libxl__initiate_device_generic_remove(egc, aodev);
return;
}
@@ -4144,36 +4144,6 @@ out:
return rc;
}
-static void libxl__initiate_device_disk_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
-{
- return libxl__initiate_device_remove(egc, aodev);
-}
-
-static void libxl__initiate_device_nic_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
-{
- return libxl__initiate_device_remove(egc, aodev);
-}
-
-static void libxl__initiate_device_vtpm_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
-{
- return libxl__initiate_device_remove(egc, aodev);
-}
-
-static void libxl__initiate_device_vkb_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
-{
- return libxl__initiate_device_remove(egc, aodev);
-}
-
-static void libxl__initiate_device_vfb_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
-{
- return libxl__initiate_device_remove(egc, aodev);
-}
-
/******************************************************************************/
/* Macro for defining device remove/destroy functions in a compact way */
@@ -4191,7 +4161,7 @@ static void libxl__initiate_device_vfb_remove(libxl__egc *egc,
* libxl_device_usbctrl_remove
* libxl_device_usbctrl_destroy
*/
-#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
+#define DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f) \
int libxl_device_##type##_##removedestroy(libxl_ctx *ctx, \
uint32_t domid, libxl_device_##type *type, \
const libxl_asyncop_how *ao_how) \
@@ -4211,13 +4181,19 @@ static void libxl__initiate_device_vfb_remove(libxl__egc *egc,
aodev->dev = device; \
aodev->callback = device_addrm_aocomplete; \
aodev->force = f; \
- libxl__initiate_device_##type##_remove(egc, aodev); \
+ libxl__initiate_device_##remtype##_remove(egc, aodev); \
\
out: \
- if (rc) return AO_CREATE_FAIL(rc); \
+ if (rc) return AO_CREATE_FAIL(rc); \
return AO_INPROGRESS; \
}
+#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
+ DEFINE_DEVICE_REMOVE_EXT(type, generic, removedestroy, f)
+
+#define DEFINE_DEVICE_REMOVE_CUSTOM(type, removedestroy, f) \
+ DEFINE_DEVICE_REMOVE_EXT(type, type, removedestroy, f)
+
/* Define all remove/destroy functions and undef the macro */
/* disk */
@@ -4242,8 +4218,8 @@ DEFINE_DEVICE_REMOVE(vtpm, remove, 0)
DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
/* usbctrl */
-DEFINE_DEVICE_REMOVE(usbctrl, remove, 0)
-DEFINE_DEVICE_REMOVE(usbctrl, destroy, 1)
+DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, remove, 0)
+DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, destroy, 1)
/* channel/console hotunplug is not implemented. There are 2 possibilities:
* 1. add support for secondary consoles to xenconsoled
@@ -4462,7 +4438,7 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
aodev->dev = dev;
aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
aodev->callback = device_complete;
- libxl__initiate_device_remove(egc, aodev);
+ libxl__initiate_device_generic_remove(egc, aodev);
break;
case LIBXL__DEVICE_KIND_QDISK:
if (--dguest->num_qdisks == 0) {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 6715c16..b7a6a13 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -679,7 +679,7 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB)
libxl__initiate_device_usbctrl_remove(egc, aodev);
else
- libxl__initiate_device_remove(egc, aodev);
+ libxl__initiate_device_generic_remove(egc, aodev);
}
}
}
@@ -778,8 +778,8 @@ out:
return;
}
-void libxl__initiate_device_remove(libxl__egc *egc,
- libxl__ao_device *aodev)
+void libxl__initiate_device_generic_remove(libxl__egc *egc,
+ libxl__ao_device *aodev)
{
STATE_AO_GC(aodev->ao);
xs_transaction_t t = 0;
@@ -809,7 +809,7 @@ void libxl__initiate_device_remove(libxl__egc *egc,
(info.paused || info.dying || info.shutdown)) {
/*
* TODO: 4.2 Bodge due to QEMU, see comment on top of
- * libxl__initiate_device_remove in libxl_internal.h
+ * libxl__initiate_device_generic_remove in libxl_internal.h
*/
rc = libxl__ev_time_register_rel(ao, &aodev->timeout,
device_qemu_timeout,
@@ -945,7 +945,7 @@ static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
!aodev->force) {
LOG(DEBUG, "Timeout reached, initiating forced remove");
aodev->force = 1;
- libxl__initiate_device_remove(egc, aodev);
+ libxl__initiate_device_generic_remove(egc, aodev);
return;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5b70c6e..839d3f1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2606,8 +2606,8 @@ _hidden void libxl__wait_device_connection(libxl__egc*,
*
* Once finished, aodev->callback will be executed.
*/
-_hidden void libxl__initiate_device_remove(libxl__egc *egc,
- libxl__ao_device *aodev);
+_hidden void libxl__initiate_device_generic_remove(libxl__egc *egc,
+ libxl__ao_device *aodev);
_hidden int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
libxl_device_usbctrl *usbctrl,
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index e35c6b5..cb25fa8 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -237,7 +237,6 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
libxl__ao_device *aodev)
{
STATE_AO_GC(aodev->ao);
- libxl_ctx *ctx = CTX;
libxl_device_usbdev *usbdevs = NULL;
int num_usbdev = 0;
int i, rc;
@@ -250,7 +249,7 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
libxl_usbctrlinfo_init(&usbctrlinfo);
usbctrl.devid = usbctrl_devid;
- rc = libxl_device_usbctrl_getinfo(ctx, domid, &usbctrl, &usbctrlinfo);
+ rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
if (rc) goto out;
if (usbctrlinfo.type != LIBXL_USBCTRL_TYPE_PV) {
@@ -277,7 +276,8 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
libxl_usbctrlinfo_dispose(&usbctrlinfo);
/* Remove usbctrl */
- return libxl__initiate_device_remove(egc, aodev);
+ libxl__initiate_device_generic_remove(egc, aodev);
+ return;
out:
libxl_device_usbctrl_dispose(&usbctrl);
@@ -287,6 +287,29 @@ out:
return;
}
+static const char * vusb_be_from_xs_fe(libxl__gc *gc, const char *fe_path,
+ uint32_t tgt_domid) {
+ const char *be_path;
+ int r;
+ uint32_t be_domid, fe_domid;
+
+ r = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/backend", fe_path),
+ &be_path);
+ if (r || !be_path) return NULL;
+
+ /* Check to see that it has the proper form, and that fe_domid ==
+ * target domid */
+ r = sscanf(be_path, "/local/domain/%d/backend/vusb/%d",
+ &be_domid, &fe_domid);
+
+ if ( r != 2 || fe_domid != tgt_domid ) {
+ LOG(ERROR, "Malformed backend, refusing to use");
+ return NULL;
+ }
+
+ return be_path;
+}
+
libxl_device_usbctrl *
libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num)
{
@@ -332,7 +355,8 @@ libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num)
})
fe_path = GCSPRINTF("%s/%s", path, *entry);
- be_path = READ_SUBPATH(fe_path, "backend");
+ be_path = vusb_be_from_xs_fe(gc, fe_path, domid);
+ if (!be_path) goto out;
usbctrl->backend_domid = READ_SUBPATH_INT(fe_path, "backend-id");
usbctrl->version = READ_SUBPATH_INT(be_path, "usb-ver");
usbctrl->ports = READ_SUBPATH_INT(be_path, "num-ports");
@@ -472,11 +496,11 @@ static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr)
!strcmp(de->d_name, ".."))
continue;
- filename = GCSPRINTF(SYSFS_USB_DEV"/%s/devnum", de->d_name);
+ filename = GCSPRINTF(SYSFS_USB_DEV "/%s/devnum", de->d_name);
if (!libxl__read_sysfs_file_contents(gc, filename, &buf, NULL))
devnum = atoi(buf);
- filename = GCSPRINTF(SYSFS_USB_DEV"/%s/busnum", de->d_name);
+ filename = GCSPRINTF(SYSFS_USB_DEV "/%s/busnum", de->d_name);
if (!libxl__read_sysfs_file_contents(gc, filename, &buf, NULL))
busnum = atoi(buf);
@@ -496,15 +520,15 @@ static int usbdev_busaddr_from_busid(libxl__gc *gc, const char *busid,
char *filename;
void *buf;
- filename = GCSPRINTF(SYSFS_USB_DEV"/%s/busnum", busid);
+ filename = GCSPRINTF(SYSFS_USB_DEV "/%s/busnum", busid);
if (!libxl__read_sysfs_file_contents(gc, filename, &buf, NULL))
- *bus = atoi((char *)buf);
+ *bus = atoi(buf);
else
return ERROR_FAIL;
- filename = GCSPRINTF(SYSFS_USB_DEV"/%s/devnum", busid);
+ filename = GCSPRINTF(SYSFS_USB_DEV "/%s/devnum", busid);
if (!libxl__read_sysfs_file_contents(gc, filename, &buf, NULL))
- *addr = atoi((char *)buf);
+ *addr = atoi(buf);
else
return ERROR_FAIL;
@@ -584,7 +608,7 @@ static bool is_usbdev_assignable(libxl__gc *gc, libxl_device_usbdev *usbdev)
usbdev->u.hostdev.hostaddr);
if (!busid) return false;
- filename = GCSPRINTF(SYSFS_USB_DEV"/%s/bDeviceClass", busid);
+ filename = GCSPRINTF(SYSFS_USB_DEV "/%s/bDeviceClass", busid);
if (libxl__read_sysfs_file_contents(gc, filename, &buf, NULL))
return false;
@@ -609,11 +633,7 @@ libxl__device_usbdev_list_for_usbctrl(libxl__gc *gc,
fe_path = GCSPRINTF("%s/device/vusb/%d",
libxl__xs_get_dompath(gc, domid), usbctrl);
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/backend", fe_path),
- &be_path);
- if (rc) goto out;
-
+ be_path = vusb_be_from_xs_fe(gc, fe_path, domid);
if (!be_path) {
rc = ERROR_FAIL;
goto out;
@@ -787,74 +807,63 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
usbdev->ctrl = usbctrl->devid;
usbdev->port = 1;
}
- } else if (!usbdev->port) {
- /* Valid port starts from 1. Choose port for us. */
- int i, ports;
+ } else {
+ /* A controller was specified; look it up */
const char *fe_path, *be_path, *tmp;
-
+
fe_path = GCSPRINTF("%s/device/vusb/%d",
- libxl__xs_get_dompath(gc, domid), usbdev->ctrl);
-
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/backend", fe_path), &be_path);
- if (rc) goto out;
+ libxl__xs_get_dompath(gc, domid),
+ usbdev->ctrl);
+ be_path = vusb_be_from_xs_fe(gc, fe_path, domid);
if (!be_path) {
rc = ERROR_FAIL;
goto out;
}
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/num-ports", be_path), &tmp);
- if (rc) goto out;
-
- ports = tmp ? atoi(tmp) : 0;
-
- for (i = 0; i < ports; i++) {
+ if (usbdev->port) {
+ /* A specific port was requested; see if it's available */
rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/port/%d", be_path, i + 1),
+ GCSPRINTF("%s/port/%d",
+ be_path, usbdev->port),
&tmp);
if (rc) goto out;
-
- if (tmp && !strcmp(tmp, "")) {
- usbdev->port = i + 1;
- break;
+
+ if (tmp && strcmp(tmp, "")) {
+ LOG(ERROR, "The controller port isn't available");
+ rc = ERROR_FAIL;
+ goto out;
}
- }
-
- if (!usbdev->port) {
- LOG(ERROR, "No available port under specified controller");
- rc = ERROR_FAIL;
- goto out;
- }
- } else {
- const char *be_path, *tmp;
-
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/device/vusb/%d/backend",
- libxl__xs_get_dompath(gc, domid),
- usbdev->ctrl),
- &be_path);
- if (rc) goto out;
-
- if (!be_path) {
- rc = ERROR_FAIL;
- goto out;
- }
+ } else {
+ /* No port was requested. Choose free port. */
+ int i, ports;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/num-ports", be_path), &tmp);
+ if (rc) goto out;
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/port/%d",
- be_path, usbdev->port),
- &tmp);
- if (rc) goto out;
+ ports = tmp ? atoi(tmp) : 0;
+
+ for (i = 0; i < ports; i++) {
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/port/%d", be_path, i + 1),
+ &tmp);
+ if (rc) goto out;
+
+ if (tmp && !strcmp(tmp, "")) {
+ usbdev->port = i + 1;
+ break;
+ }
+ }
- if (tmp && strcmp(tmp, "")) {
- LOG(ERROR, "The controller port isn't available");
- rc = ERROR_FAIL;
- goto out;
+ if (!usbdev->port) {
+ LOG(ERROR, "No available port under specified controller");
+ rc = ERROR_FAIL;
+ goto out;
+ }
}
}
-
+
rc = 0;
out:
@@ -966,7 +975,7 @@ static int usbintf_get_drvpath(libxl__gc *gc, const char *intf, char **drvpath)
struct stat st;
int rc;
- spath = GCSPRINTF(SYSFS_USB_DEV"/%s/driver", intf);
+ spath = GCSPRINTF(SYSFS_USB_DEV "/%s/driver", intf);
rc = lstat(spath, &st);
if (rc == 0) {
@@ -1012,7 +1021,7 @@ static int unbind_usbintf(libxl__gc *gc, const char *intf)
{
char *path;
- path = GCSPRINTF(SYSFS_USB_DEV"/%s/driver/unbind", intf);
+ path = GCSPRINTF(SYSFS_USB_DEV "/%s/driver/unbind", intf);
return sysfs_write_intf(gc, intf, path);
}
@@ -1037,7 +1046,7 @@ static int usbintf_is_assigned(libxl__gc *gc, char *intf)
int rc;
struct stat st;
- spath = GCSPRINTF(SYSFS_USBBACK_DRIVER"/%s", intf);
+ spath = GCSPRINTF(SYSFS_USBBACK_DRIVER "/%s", intf);
rc = lstat(spath, &st);
if (rc == 0)
@@ -1157,7 +1166,7 @@ static int usbback_dev_unassign(libxl__gc *gc, const char *busid)
* handle it later.
*/
usbintf_encode = usb_interface_xenstore_encode(gc, intf);
- path = GCSPRINTF(USBBACK_INFO_PATH"/%s/%s/driver_path",
+ path = GCSPRINTF(USBBACK_INFO_PATH "/%s/%s/driver_path",
usbdev_encode, usbintf_encode);
rc = libxl__xs_read_checked(gc, XBT_NULL, path, &drvpath);
if (rc) continue;
@@ -1167,7 +1176,7 @@ static int usbback_dev_unassign(libxl__gc *gc, const char *busid)
}
/* finally, remove xenstore driver path */
- path = GCSPRINTF(USBBACK_INFO_PATH"/%s", usbdev_encode);
+ path = GCSPRINTF(USBBACK_INFO_PATH "/%s", usbdev_encode);
libxl__xs_rm_checked(gc, XBT_NULL, path);
rc = 0;
@@ -1210,7 +1219,7 @@ static int usbback_dev_assign(libxl__gc *gc, const char *busid)
char *path;
usbintf_encode = usb_interface_xenstore_encode(gc, intf);
- path = GCSPRINTF(USBBACK_INFO_PATH"/%s/%s/driver_path",
+ path = GCSPRINTF(USBBACK_INFO_PATH "/%s/%s/driver_path",
usbdev_encode, usbintf_encode);
if (libxl__xs_write_checked(gc, XBT_NULL, path, drvpath) < 0)
goto out;
@@ -1494,11 +1503,10 @@ int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx,
dompath = libxl__xs_get_dompath(gc, domid);
- rc = libxl__xs_read_checked(gc, XBT_NULL,
- GCSPRINTF("%s/device/vusb/%d/backend", dompath, ctrl),
- &be_path);
- if (rc) goto out;
-
+ be_path = vusb_be_from_xs_fe(gc,
+ GCSPRINTF("%s/device/vusb/%d/backend",
+ dompath, ctrl),
+ domid);
if (!be_path) {
rc = ERROR_FAIL;
goto out;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2015-12-10 12:08 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-10 12:05 [PATCH v10 0/5] xen pvusb toolstack work George Dunlap
2015-12-10 12:05 ` [PATCH v10 1/5] libxl: export some functions for pvusb use George Dunlap
2015-12-10 12:05 ` [PATCH v10 2/5] libxl_utils: add internal function to read sysfs file contents George Dunlap
2015-12-10 12:05 ` [PATCH v10 3/5] libxl: add pvusb API George Dunlap
2015-12-10 12:08 ` George Dunlap [this message]
2015-12-14 7:25 ` Chun Yan Liu
2015-12-14 11:37 ` George Dunlap
2015-12-15 2:53 ` Chun Yan Liu
2015-12-10 12:05 ` [PATCH v10 4/5] xl: add pvusb commands George Dunlap
2015-12-10 12:05 ` [PATCH v10 5/5] domcreate: support pvusb in configuration file George Dunlap
2015-12-10 12:16 ` [PATCH v10 0/5] xen pvusb toolstack work George Dunlap
2015-12-10 12:23 ` George Dunlap
2015-12-11 3:51 ` Chun Yan Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56696B4B.7060801@citrix.com \
--to=george.dunlap@citrix.com \
--cc=caobosimon@gmail.com \
--cc=cyliu@suse.com \
--cc=george.dunlap@eu.citrix.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@citrix.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.