>>> On 12/10/2015 at 08:08 PM, in message <56696b4b.7060...@citrix.com>, George
Dunlap <george.dun...@citrix.com> wrote:
> On 10/12/15 12:05, George Dunlap wrote:
> > From: Chunyan Liu <cy...@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 <cy...@suse.com>
> > Signed-off-by: Simon Cao <caobosi...@gmail.com>
> > Signed-off-by: George Dunlap <george.dun...@citrix.com>
>
> Attached is a diff of v9 -> v10 for convenience.
Thanks very much, George!
I've applied your new patch and tested, there are a couple of changes needed to
get tests PASSED. A small extra patch is written on top of your new patch, as in
attachment, please have a look.
Otherwise, I agreed with all your other changes. It's great improvement.
Thanks a lot!
>
> One remaining question I had regarding this patch...
For the question, see below.
>
> > +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?
Calculating in above way is to allocate enough space for d_name, whereas
"struct dirent de_buf" won't allocate space for d_name (which is char *).
Codes for calling read_dir_r are often done like above.
- Chunyan
>
> If it is necessary, it would be better to have it inside a function or
> macro called "alloc_dirent" or something like that.
>
> -George
>
>
>From 771c99218a4704eb6a4850dfafb1cafad0798b9d Mon Sep 17 00:00:00 2001
From: Chunyan Liu <cy...@suse.com>
Date: Mon, 14 Dec 2015 15:00:50 +0800
Subject: [PATCH 4/6] libxl pvusb API changes
* format fix: extra white space, line > 80, etc.
* return ERROR_FAILED instead of errno (>0) in sysfs_write_intf
* fix an error in libxl_ctrlport_to_device_usbdev
Signed-off-by: Chunyan Liu <cy...@suse.com>
---
tools/libxl/libxl_pvusb.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index cb25fa8..5f189d6 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -287,14 +287,15 @@ out:
return;
}
-static const char * vusb_be_from_xs_fe(libxl__gc *gc, const char *fe_path,
- uint32_t tgt_domid) {
+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);
+ &be_path);
if (r || !be_path) return NULL;
/* Check to see that it has the proper form, and that fe_domid ==
@@ -302,11 +303,11 @@ static const char * vusb_be_from_xs_fe(libxl__gc *gc,
const char *fe_path,
r = sscanf(be_path, "/local/domain/%d/backend/vusb/%d",
&be_domid, &fe_domid);
- if ( r != 2 || fe_domid != tgt_domid ) {
+ if (r != 2 || fe_domid != tgt_domid) {
LOG(ERROR, "Malformed backend, refusing to use");
return NULL;
}
-
+
return be_path;
}
@@ -810,7 +811,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
} 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);
@@ -828,7 +829,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
be_path, usbdev->port),
&tmp);
if (rc) goto out;
-
+
if (tmp && strcmp(tmp, "")) {
LOG(ERROR, "The controller port isn't available");
rc = ERROR_FAIL;
@@ -837,7 +838,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
} 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;
@@ -849,7 +850,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
GCSPRINTF("%s/port/%d", be_path, i
+ 1),
&tmp);
if (rc) goto out;
-
+
if (tmp && !strcmp(tmp, "")) {
usbdev->port = i + 1;
break;
@@ -863,7 +864,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
}
}
}
-
+
rc = 0;
out:
@@ -1006,7 +1007,7 @@ static int sysfs_write_intf(libxl__gc *gc, const char
*intf, const char *path)
close(fd);
if (rc < 0) {
LOGE(ERROR, "write '%s' to '%s' failed", intf, path);
- return errno;
+ return ERROR_FAIL;
}
if (rc != strlen(intf)) {
LOG(ERROR, "write '%s' to '%s' failed: incorrect write count",
@@ -1498,15 +1499,14 @@ int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx,
libxl_device_usbdev *usbdev)
{
GC_INIT(ctx);
- const char *dompath, *be_path, *busid;
+ const char *dompath, *fe_path, *be_path, *busid;
int rc;
dompath = libxl__xs_get_dompath(gc, domid);
- be_path = vusb_be_from_xs_fe(gc,
- GCSPRINTF("%s/device/vusb/%d/backend",
- dompath, ctrl),
- domid);
+ fe_path = GCSPRINTF("%s/device/vusb/%d", dompath, ctrl);
+
+ be_path = vusb_be_from_xs_fe(gc, fe_path, domid);
if (!be_path) {
rc = ERROR_FAIL;
goto out;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
