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

[Stefan Hajnoczi]

On Thu, Nov 17, 2011 at 11:34 AM, Chun Yan Liu <address@hidden> wrote:
> Thanks for your suggestions.
>
> For the usage "qemu-nbd -f disk.img", adding some code could implement it. I
> think it could be like "losetup -f" usage.
>
> #qemu-nbd -f
>
> show the first free nbd device at this moment.
>
> user can choose to issue "qemu-nbd -c THAT_DEVICE disk.img" or not.
>
> #qemu-nbd -f disk.img
>
> find a free nbd device and connect disk.img to that device.
>
> How do you think?
>
> For the race conditions caused by executing multiple qemu-nbd -f at the same
> time, I've tried both ways (1. lock; 2. if one device not work, trying other
> devices until one works).
>
> In my testing, the 2nd way has problem. When issuing "qemu-nbd -c /dev/nbd0
> disk.img -v" and "qemu-nbd -c /dev/nbd0 disk1.img -v" at the same time, the
> latter one will eventually exit with EXIT_FAILURE, but the first one cannot
> work normally as well, it cannot show disk partitions. Executing multiple
> "qemu-nbd -f" has same problem.

The problem is that qemu/nbd.c is doing it wrong.

The linux/drivers/block/nbd.c driver returns -EBUSY from
ioctl(NBD_SET_SOCK) if the device already has a socket configured.
However, qemu/nbd.c issues NBD_SET_BLKSIZE and other settings ioctls
*before* NBD_SET_SOCK.

This clobbers the existing nbd connection settings.  Then we take over
the device using NBD_CLEAR_SOCK and NBD_SET_SOCK instead of noticing
there is already a socket configured.

I think qemu/nbd.c:nbd_init() should be fixed to:

1. NBD_SET_SOCK.  This fails with -EBUSY if an existing socket is
configured.  This is the atomic acquire/test operation.
2. NBD_SET_BLKSIZE and other settings ioctls.

The only user-visible change is that qemu-nbd -c /dev/nbd0 no longer
hijacks the nbd device.  Instead it properly detects -EBUSY.  This
seems like a reasonable change to make although we could restrict it
to only qemu-nbd -f disk.img in order to preserve the current behavior
for qemu-nbd -c /dev/nbd0.

I don't think using file locks is necessary since it can be done
properly through the nbd ioctl interface.

Stefan