Re: [PATCH v14 03/10] usbip: exporting devices: new connect operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
> Implementation of new connect operation. This is linked as a part of 
> usbip command. With this patch, usbip command has following operations.
> 
>  bind
>  unbind
>  list (local/remote)
>  attach
>  detach
>  port
>  connect ... this patch

Don't include existing commands. Just list the new one. Why do you call
this connect. Isn't it really export - connect isn't accurate. Call it
export/unexport.

> 
> In device side node, this binds a device internally, sends an export 
> request and receives export response. The definition of the request and 
> response are defined in original code: tools/usb/usbip/usbip_network.h 
> but was not used. They are corresponding to NEW-3 in following diagram. 
> 
> EXISTING) - invites devices from application(vhci)-side
>          +------+                               +------------------+
>  device--+ STUB |                               | application/VHCI |
>          +------+                               +------------------+
>          (server)                               (client)
>  1) # usbipd ... start daemon
>  = = =
>  2) # usbip list --local
>  3) # usbip bind
>                   <--- list bound devices ---  4) # usbip list --remote
>                   <--- import a device ------  5) # usbip attach
>  = = =
>                      X disconnected            6) # usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stub)-side
>          +------+                               +------------------+
>  device--+ STUB |                               | application/VHCI |
>          +------+                               +------------------+
>          (client)                               (server)

Make the left side server and right side client. I think you might be
using server and client network terminology. I would like to see server
as the system that has the device physically attached to.


I still want to see server as the one that is connected to the device.
It is just that in this new proposed model, server is exporting devices.

Also does usbip run here in user mode. Can any user run uspip and initiate
export? If so, I don't think I can take this patch series. I don't like to
see non root being able to export and/or make attached devices public.

>                                             1) # usbipa ... start daemon

Did you mean uspipd or is uspipa a new daemon? Okay now I see that
there is new daemon introduced in patch 7/10 - Why do you need to
add a new daemon? Why not add it to usbipd instead?

I would like to see all of this information in README. Including this
in every single patch is confusing. btw you are also missing doc changes
for these new options you are adding.

When you change README, please include the both server and client end
commands just like README does now.

>  = = =
>  2) # usbip list --local
>  3) # usbip connect    --- export a device ------>
>  = = =
>  4) # usbip disconnect --- un-export a device --->
> 
>  Bind and unbind are done in connect and disconnect internally.
> 
> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> ---
>  tools/usb/usbip/src/Makefile.am     |   3 +-
>  tools/usb/usbip/src/usbip.c         |   7 +
>  tools/usb/usbip/src/usbip.h         |   3 +
>  tools/usb/usbip/src/usbip_connect.c | 212 ++++++++++++++++++++++++++++
>  4 files changed, 224 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am
> index e81a4eb..0947476 100644
> --- a/tools/usb/usbip/src/Makefile.am
> +++ b/tools/usb/usbip/src/Makefile.am
> @@ -6,6 +6,7 @@ sbin_PROGRAMS := usbip usbipd
>  
>  usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
>  		 usbip_attach.c usbip_detach.c usbip_list.c \
> -		 usbip_bind.c usbip_unbind.c usbip_port.c
> +		 usbip_bind.c usbip_unbind.c usbip_port.c \
> +		 usbip_connect.c
>  
>  usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
> diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c
> index d7599d9..ad2a259 100644
> --- a/tools/usb/usbip/src/usbip.c
> +++ b/tools/usb/usbip/src/usbip.c
> @@ -4,6 +4,7 @@
>   *
>   * Copyright (C) 2011 matt mooney <mfm@xxxxxxxxxxxxx>
>   *               2005-2007 Takahiro Hirofuchi
> + * Copyright (C) 2015-2016 Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
>   *
>   * This program is free software: you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -76,6 +77,12 @@ static const struct command cmds[] = {
>  		.usage = usbip_detach_usage
>  	},
>  	{
> +		.name  = "connect",
> +		.fn    = usbip_connect,
> +		.help  = "Connect a USB device to a remote computer",
> +		.usage = usbip_connect_usage
> +	},
> +	{
>  		.name  = "list",
>  		.fn    = usbip_list,
>  		.help  = "List exportable or local USB devices",
> diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h
> index c296910..3c22c27 100644
> --- a/tools/usb/usbip/src/usbip.h
> +++ b/tools/usb/usbip/src/usbip.h
> @@ -1,6 +1,7 @@
>  /*
>   * Copyright (C) 2011 matt mooney <mfm@xxxxxxxxxxxxx>
>   *               2005-2007 Takahiro Hirofuchi
> + * Copyright (C) 2015-2016 Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
>   *
>   * This program is free software: you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -30,12 +31,14 @@ int usbip_list(int argc, char *argv[]);
>  int usbip_bind(int argc, char *argv[]);
>  int usbip_unbind(int argc, char *argv[]);
>  int usbip_port_show(int argc, char *argv[]);
> +int usbip_connect(int argc, char *argv[]);
>  
>  void usbip_attach_usage(void);
>  void usbip_detach_usage(void);
>  void usbip_list_usage(void);
>  void usbip_bind_usage(void);
>  void usbip_unbind_usage(void);
> +void usbip_connect_usage(void);
>  
>  int usbip_bind_device(char *busid);
>  int usbip_unbind_device(char *busid);
> diff --git a/tools/usb/usbip/src/usbip_connect.c b/tools/usb/usbip/src/usbip_connect.c
> new file mode 100644
> index 0000000..73668db2
> --- /dev/null
> +++ b/tools/usb/usbip/src/usbip_connect.c
> @@ -0,0 +1,212 @@
> +/*
> + * Copyright (C) 2011 matt mooney <mfm@xxxxxxxxxxxxx>
> + *               2005-2007 Takahiro Hirofuchi
> + * Copyright (C) 2015-2016 Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <sys/stat.h>
> +
> +#include <limits.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <getopt.h>
> +#include <unistd.h>
> +
> +#include "usbip_host_driver.h"
> +#include "usbip_host_common.h"
> +#include "usbip_device_driver.h"
> +#include "usbip_common.h"
> +#include "usbip_network.h"
> +#include "usbip.h"
> +
> +static struct usbip_host_driver *driver = &host_driver;
> +
> +static const char usbip_connect_usage_string[] =
> +	"usbip connect <args>\n"
> +	"    -r, --remote=<host>    Address of a remote computer\n"
> +	"    -b, --busid=<busid>    Bus ID of a device to be connected\n"
> +	"    -d, --device           Run with an alternate driver, e.g. vUDC\n";
> +
> +void usbip_connect_usage(void)
> +{
> +	printf("usage: %s", usbip_connect_usage_string);
> +}
> +
> +static int send_export_device(int sockfd, struct usbip_usb_device *udev)
> +{
> +	int rc;
> +	struct op_export_request request;
> +	uint16_t code = OP_REP_EXPORT;
> +
> +	/* send a request */
> +	rc = usbip_net_send_op_common(sockfd, OP_REQ_EXPORT, 0);
> +	if (rc < 0) {
> +		err("send op_common");
> +		return -1;
> +	}
> +
> +	memset(&request, 0, sizeof(request));
> +	memcpy(&request.udev, udev, sizeof(request.udev));
> +
> +	PACK_OP_EXPORT_REQUEST(0, &request);
> +
> +	rc = usbip_net_send(sockfd, (void *) &request, sizeof(request));
> +	if (rc < 0) {
> +		err("send op_export_request");
> +		return -1;
> +	}
> +
> +	/* receive a reply */
> +	rc = usbip_net_recv_op_common(sockfd, &code);
> +	if (rc < 0) {
> +		err("recv op_common");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int export_device(char *busid, int sockfd)
> +{
> +	int rc;
> +	struct usbip_exported_device *edev;
> +
> +	rc = usbip_driver_open(driver);
> +	if (rc) {
> +		err("open driver");
> +		goto err_out;
> +	}
> +
> +	rc = usbip_refresh_device_list(driver);
> +	if (rc < 0) {
> +		err("could not refresh device list");
> +		goto err_driver_close;
> +	}
> +
> +	edev = usbip_get_device(driver, busid);
> +	if (edev == NULL) {
> +		err("find device");
> +		goto err_driver_close;
> +	}
> +
> +	rc = send_export_device(sockfd, &edev->udev);
> +	if (rc < 0) {
> +		err("send export");
> +		goto err_driver_close;
> +	}
> +
> +	rc = usbip_export_device(edev, sockfd);
> +	if (rc < 0) {
> +		err("export device");
> +		goto err_driver_close;
> +	}
> +
> +	usbip_driver_close(driver);
> +
> +	return 0;
> +
> +err_driver_close:
> +	usbip_driver_close(driver);
> +err_out:
> +	return -1;
> +}
> +
> +static int connect_device(char *host, char *busid, int bind)
> +{
> +	int sockfd;
> +	int rc;
> +
> +	if (bind) {
> +		rc = usbip_bind_device(busid);
> +		if (rc) {
> +			err("bind");
> +			goto err_out;
> +		}
> +	}
> +
> +	sockfd = usbip_net_tcp_connect(host, usbip_port_string);
> +	if (sockfd < 0) {
> +		err("tcp connect");
> +		goto err_unbind_device;
> +	}
> +
> +	rc = export_device(busid, sockfd);
> +	if (rc < 0) {
> +		err("export");
> +		goto err_close_conn;
> +	}
> +
> +	close(sockfd);
> +
> +	return 0;
> +err_close_conn:
> +	close(sockfd);
> +err_unbind_device:
> +	if (bind)
> +		usbip_unbind_device(busid);
> +err_out:
> +	return -1;
> +}
> +
> +int usbip_connect(int argc, char *argv[])
> +{
> +	static const struct option opts[] = {
> +		{ "remote", required_argument, NULL, 'r' },
> +		{ "busid",  required_argument, NULL, 'b' },
> +		{ "device", no_argument,       NULL, 'd' },
> +		{ NULL, 0,  NULL, 0 }
> +	};
> +	char *host = NULL;
> +	char *busid = NULL;
> +	int opt;
> +	int bind = 1;
> +	int ret = -1;
> +
> +	for (;;) {
> +		opt = getopt_long(argc, argv, "r:b:d", opts, NULL);
> +
> +		if (opt == -1)
> +			break;
> +
> +		switch (opt) {
> +		case 'r':
> +			host = optarg;
> +			break;
> +		case 'b':
> +			busid = optarg;
> +			break;
> +		case 'd':
> +			driver = &device_driver;
> +			bind = 0;
> +			break;
> +		default:
> +			goto err_out;
> +		}
> +	}
> +
> +	if (!host || !busid)
> +		goto err_out;
> +
> +	ret = connect_device(host, busid, bind);
> +	goto out;
> +
> +err_out:
> +	usbip_connect_usage();
> +out:
> +	return ret;
> +}
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux