On Fri, Nov 25, 2011 at 10:19 AM, Chunyan Liu <
address@hidden> wrote:
>
>
> 2011/11/24 Stefan Hajnoczi <
address@hidden>
>>
>> On Thu, Nov 24, 2011 at 3:38 AM, Chunyan Liu <
address@hidden> wrote:
>> >
>> >
>> > 2011/11/23 Stefan Hajnoczi <
address@hidden>
>> >>
>> >> On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <
address@hidden> wrote:
>> >> > V3:
>> >> > Remove file lock in main().
>> >> > Try to find new free nbd device and connect to it if connecting to
>> >> > the
>> >> > first
>> >> > first found free nbd device failed.
>> >> >
>> >> > Signed-off-by: Chunyan Liu <
address@hidden>
>> >> > ---
>> >> > qemu-nbd.c | 80
>> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >> > 1 files changed, 79 insertions(+), 1 deletions(-)
>> >>
>> >> I not seeing the part where you adjusted the ioctl order.
>> >>
>> >> The /proc/partitions scanning is unnecessary since we can just loop
>> >> over /dev/ndb%d and try to initialize. If a device is in use then
>> >> init will fail and we need to try the next one. If a device is free
>> >> we can continue with normal operation. I guess I'm saying that once
>> >> you fix the ioctl order then there's no need for another mechanism to
>> >> test whether or not a device is in use.
>> >
>> > The way of scanning /proc/partitions to find an unused nbd device first
>> > can
>> > borrow the code for "qemu-nbd -c" to do the left things. .
>> > The way of loop over /dev/nbd%d and try to initialize, from the first
>> > thought, needs do all things in the loop, including handling -v,
>> > nbd_init,
>> > nbd_client, etc. That part of code is quite similar to "qemu-nbd -c". I
>> > don't know if that is better?
>>
>> This might be a chance to refactor the code slightly, that would also
>> avoid you having to introduce a goto retry.
>>
> About detail implementation of the loop over /dev/nbd%d way, have done some
> coding and testing. I'm afraid the reorganization work is not very slight,
> not sure how do you think.
> First, through nbd_init() success or fail to check if the device is in use,
> at least nbd_init() and operations before nbd_init() but device relative
> should be in the loop. Usually, there are two ways to be considered:
> 1. Try to divide current "qemu-nbd -c" processing code into two parts (
> nbd_init() and before, and operations after nbd_init()), 1st part put into
> the loop, 2nd part outside loop. This way is hard to achieve. nbd_init()
> should be in client thread, but the loop is outside that thread, it's not
> proper to drag out nbd_init() from client thread and put it into the loop.
> 2. Put all "qemu-nbd -c" processing code (device relative) in the loop,
> directly check the "qemu-nbd -c /dev/nbd%d disk.img" result, if fail, try
> next nbd device. In this way, will adjust current code order and extract
> device related codes to a new function so that both "qemu-nbd -c" and
> "qemu-nbd -f" can use.(some initialization work and cleanup work to be
> cared)
>
> A draft is in attachment. qemu-nbd -f is working, since I'm not sure such
> change is acceptable or not, not extract code in the loop into a function
> yet.