Re: [Qemu-devel] [PATCH 27/27] vhost-user-scsi: remove server_sock from VusDev

[Philippe Mathieu-Daudé]

Hi Marc-André,

On 08/23/2017 01:20 PM, Marc-André Lureau wrote:

It is unneeded in the VusDev device structure, and also simplify a bit
the code.

Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 52 ++++++++++++++-----------------
  1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index cfd62b46ce..3166331856 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -28,7 +28,6 @@ typedef struct VusIscsiLun {
  typedef struct VusDev {
      VugDev parent;

- int server_sock;

      VusIscsiLun lun;
  } VusDev;

@@ -371,48 +370,30 @@ fail: static void vdev_scsi_free(VusDev *vdev_scsi)

let's kill vdev_scsi_free(),

  {
-    if (vdev_scsi->server_sock >= 0) {
-        close(vdev_scsi->server_sock);
-    }
      g_free(vdev_scsi);
  }

-static VusDev *vdev_scsi_new(int server_sock)

+static VusDev *vdev_scsi_new(void)
  {
-    VusDev *vdev_scsi;
-
-    assert(server_sock >= 0);
-
-    vdev_scsi = g_new0(VusDev, 1);
-    vdev_scsi->server_sock = server_sock;
-
-    return vdev_scsi;
+    return g_new0(VusDev, 1);

also kill vdev_scsi_new() ...

  }

-static int vdev_scsi_run(VusDev *vdev_scsi)

+static int vdev_scsi_run(VusDev *vdev_scsi, int sock)
  {
      GMainLoop *loop;
      GIOChannel *chan;
-    int cli_sock;
      int ret = 0;

assert(vdev_scsi);

-    assert(vdev_scsi->server_sock >= 0);
-
-    cli_sock = accept(vdev_scsi->server_sock, NULL, NULL);
-    if (cli_sock < 0) {
-        perror("accept");
-        return -1;
-    }

loop = g_main_loop_new(NULL, FALSE);

      vug_init(&vdev_scsi->parent,
-             cli_sock,
+             sock,
               loop,
               vus_panic_cb,
               &vus_iface);

- chan = g_io_channel_unix_new(cli_sock);

+    chan = g_io_channel_unix_new(sock);
      g_io_add_watch(chan, G_IO_IN, vus_vhost_cb, vdev_scsi);
      g_main_loop_run(loop);
      g_io_channel_unref(chan);
@@ -428,7 +409,7 @@ int main(int argc, char **argv)
      VusDev *vdev_scsi = NULL;
      char *unix_fn = NULL;
      char *iscsi_uri = NULL;
-    int sock, opt, err = EXIT_SUCCESS;
+    int lsock = -1, csock = -1, opt, err = EXIT_SUCCESS;

while ((opt = getopt(argc, argv, "u:i:")) != -1) {

          switch (opt) {
@@ -448,17 +429,24 @@ int main(int argc, char **argv)
          goto help;
      }

- sock = unix_sock_new(unix_fn);

-    if (sock < 0) {
+    lsock = unix_sock_new(unix_fn);
+    if (lsock < 0) {
+        goto err;
+    }
+
+    csock = accept(lsock, NULL, NULL);
+    if (csock < 0) {
+        perror("accept");
          goto err;
      }
-    vdev_scsi = vdev_scsi_new(sock);
+
+    vdev_scsi = vdev_scsi_new();

... and use g_new0() directly:

       vdev_scsi = g_new0(VusDev, 1);

if (vus_iscsi_add_lun(&vdev_scsi->lun, iscsi_uri) != 0) {

          goto err;
      }

- if (vdev_scsi_run(vdev_scsi) != 0) {

+    if (vdev_scsi_run(vdev_scsi, csock) != 0) {
          goto err;
      }

@@ -467,6 +455,12 @@ out:

          vdev_scsi_free(vdev_scsi);

and g_free() here:

            g_free(vdev_scsi);

If you agree:
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>

          unlink(unix_fn);
      }
+    if (csock >= 0) {
+        close(csock);
+    }
+    if (lsock >= 0) {
+        close(lsock);
+    }
      g_free(unix_fn);
      g_free(iscsi_uri);