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

Re: [libvirt] [PATCH 05/13] qemu: support NBD with Unix sockets



On 2013年03月11日 21:20, Paolo Bonzini wrote:
Il 11/03/2013 10:34, Osier Yang ha scritto:

+    if (STRPREFIX(host, "unix:/")) {
+        src = strchr(host + strlen("unix:"), ':');
+        if (src)
+            *src++ = '\0';

Same comments as previous patches.

I think in this case the value that is assigned to src is complex enough
that it's better to keep it separate.

Okay, but still applies to other similar codes.



-    *port++ = '\0';
-    h->name = strdup(host);
-    if (!h->name)
-        goto no_memory;
+        h->transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX;
+        h->socket = strdup(host + strlen("unix:"));
+    } else {
+        port = strchr(host, ':');
+        if (!port) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse nbd filename '%s'"),
disk->src);
+            goto error;
+        }

-    src = strchr(port, ':');
-    if (src)
-        *src++ = '\0';
+        *port++ = '\0';
+        h->name = strdup(host);
+        if (!h->name)
+            goto no_memory;

-    h->port = strdup(port);
-    if (!h->port)
-        goto no_memory;
+        src = strchr(port, ':');
+        if (src)
+            *src++ = '\0';
+
+        h->port = strdup(port);
+        if (!h->port)
+            goto no_memory;
+    }

       if (src&&   STRPREFIX(src, "exportname=")) {
           src = strdup(strchr(src, '=') + 1);
@@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk,
virBufferPtr opt)
           virBufferEscape(opt, ',', ",", ":%s",
                           disk->hosts->port ? disk->hosts->port :
"10809");
           break;
+    case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
+        if (!disk->hosts->socket) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("socket attribute required for unix
transport"));
+            return -1;
+        }

Not sure if we should do this when parsing, as I think the "socket" is
required as long as the transport protocol is "unix", unless it's
generated somewhere.

Yeah, we cannot be sure in general that it will be required for _all_
protocols, so I kept it here and followed what gluster does.


Agreed.


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