Re: [Qemu-devel] [PATCH V2] Add -f option to qemu-nbd

[Chun Yan Liu]

I've had a look at nbd driver code and viewed the trace log, and get clear about why the previously mentioned problem happens:

1st time: qemu-nbd -c /dev/nbd0 disk.img

nbd_init: send these ioctl(s) in order, SET_BLKSIZE, SET_SIZE, CLEAR_SOCK, SET_SOCK

nbd_clinet: NBD_DO_IT (it will then handle request(s) in which it should use nbd_device->sock.)

2st time: qemu-nbd -c /dev/nbd0 disk1.img

nbd_init: send same ioctl(s) to the same nbd device, it will reset nbd_device->sock

nbd_client: still send NBD_DO_IT, it find there is on client connected, then return EBUSY and send CLEAR_SOCK, the result is: it will clear ndb_device->sock, which will cause the 1st time "qemu-nbd -c" fail to handle request any longer, including unable to read partition table.

According to above code logic, if lock in an early place is not accepted, then removing CLEAR_SOCK in nbd_init phase can also solve problem. In fact, if cleanup work done well, I think that ioctl is not needed. Any comments?
 

diff --git a/nbd.c b/nbd.c

index e6c931c..067a57b 100644

--- a/nbd.c

+++ b/nbd.c

@@ -386,15 +386,6 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)

         return -1;

     }

-    TRACE("Clearing NBD socket");

-

-    if (ioctl(fd, NBD_CLEAR_SOCK) == -1) {

-        int serrno = errno;

-        LOG("Failed clearing NBD socket");

-        errno = serrno;

-        return -1;

-    }

-

     TRACE("Setting NBD socket");

     if (ioctl(fd, NBD_SET_SOCK, csock) == -1) {

>>> Paolo Bonzini <address@hidden> 11/18/2011 4:59 PM >>>
On 11/18/2011 02:25 AM, Chun Yan Liu wrote:
> Yes. I have tested using same device twice as described in my previous
> mail. Without lock:
>
> If issuing "qemu-nbd -c /dev/nbd0 disk.img" and "qemu-nbd -c  /dev/nbd0
> disk1.img" almost at the same time, both can pass nbd_init() and get to
> nbd_client(), then the latter one will fail and exit, but the first one
> does not work well either (fail to show partitions.) That's why I think
> we should add a lock in an earlier time.

This is an initialization problem.  As Stefan wrote, functionality for
atomic acquisition of NBD devices is provided by the kernel; the problem
is simply that QEMU does not use it. :)

Paolo