2011/11/29 Stefan Hajnoczi <stefa...@gmail.com>

> On Fri, Nov 25, 2011 at 10:19 AM, Chunyan Liu <cy...@suse.com> wrote:
> >
> >
> > 2011/11/24 Stefan Hajnoczi <stefa...@gmail.com>
> >>
> >> On Thu, Nov 24, 2011 at 3:38 AM, Chunyan Liu <cy...@suse.com> wrote:
> >> >
> >> >
> >> > 2011/11/23 Stefan Hajnoczi <stefa...@gmail.com>
> >> >>
> >> >> On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cy...@suse.com>
> 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 <cy...@suse.com>
> >> >> > ---
> >> >> >  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.
>
> I think a common function makes sense.  This may be nicest in two
> patches, Patch 1 moves the code into a function, Patch 2 implements
> -f/find.
>
>
Just sent the patch sets according to above suggestion. Please review.
1/3-Update-ioctl-order-in-nbd_init-to-detect-EBUSY
2/3-Extract-code-to-nbd_setup-function-to-be-used-for-mamy-purposes
3/3-Add-f-option-to-qemu-nbd

Stefan
>
>

Reply via email to