diff mbox

[V2] Add -f option to qemu-nbd

Message ID 4ECD32BF0200006600006F28@novprvlin0050.provo.novell.com
State New
Headers show

Commit Message

Chunyan Liu Nov. 23, 2011, 6:51 a.m. UTC 
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? 
  

>>> Paolo Bonzini <pbonzini@redhat.com> 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

Comments

Paolo Bonzini Nov. 23, 2011, 7:24 a.m. UTC | #1 
On 11/23/2011 07:51 AM, Chun Yan Liu wrote:
>
> 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?

I think you're right.  In addition, SET_BLKSIZE and SET_SIZE should not 
be sent unless SET_SOCK succeeds.

Paolo
diff mbox

Patch

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) {