On 12/26/2016 08:08 AM, Nobuo Iwata wrote:
> Implementation of new disconnect 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 ... previous patch
>  disconnect ... this patch
> 
> In device side node, this sends an un-export request, receives an 
> un-export response and unbind corresponding device internally. The 
> definition of the request and response are defined in original code: 
> tools/usb/usbip/usbip_network.h but was not used. It's corresponding to 
> NEW-4 in following diagram.
> 
> To find disconnecting device, requesting host and requested 
> bus-id-in-requester identifies the target. So it cannot to disconnect a 
> device from other host than a host which connected the device. 
> 
> 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)
>                                             1) # usbipa ... start daemon
>  = = =
>  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.

The same as in previous commit. Please clarify that those operations are
done only for stub driver.

(...)

> +static int disconnect_device(char *host, char *busid, int unbind)
> +{
> +     int sockfd;
> +     int rc;
> +
> +     sockfd = usbip_net_tcp_connect(host, usbip_port_string);
> +     if (sockfd < 0) {
> +             err("tcp connect");
> +             return -1;
> +     }
> +
> +     rc = unexport_device(busid, sockfd);
> +     if (rc < 0) {
> +             err("unexport");
> +             close(sockfd);
> +             return -1;
> +     }
> +
> +     close(sockfd);

My comment from previous version (still not fixed):

>> +
>> +    rc = unexport_device(busid, sockfd);
>> +    if (rc < 0) {
>> +            err("unexport");
>> +            close(sockfd);
>> +            return -1;
>> +    }
>
> close(sockfd) in case of error, otherwise close(sockfd)...
>
> Probably you can place close(sockfd) above if to avoid this weird
> copy-paste.
>
>> +
>> +    close(sockfd);
>> +


> +
> +     if (unbind) {
> +             rc = usbip_unbind_device(busid);
> +             if (rc) {
> +                     err("unbind");
> +                     return -1;
> +             }
> +     }
> +
> +     return 0;
> +}
> +
> +int usbip_disconnect(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 unbind = 1;

Shouldn't this parameter be exposed to cmd line among remote, busid and
device? User may want to disconnect device but leave stub driver bound
to it.

> +     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;
> +                     unbind = 0;
> +                     break;
> +             default:
> +                     goto err_out;
> +             }
> +     }
> +
> +     if (!host || !busid)
> +             goto err_out;
> +
> +     ret = disconnect_device(host, busid, unbind);
> +     goto out;
> +
> +err_out:
> +     usbip_disconnect_usage();
> +out:
> +     return ret;
> +}
> 

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to