Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-28 Thread Zhi Yong Wu
On Wed, Sep 21, 2011 at 5:37 PM, Ronnie Sahlberg wrote: > This provides built-in support for iSCSI to QEMU. > This has the advantage that the iSCSI devices need not be made visible to the > host, which is useful if you have very many virtual machines and very many > iscsi devices. > It also has

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-25 Thread Kevin Wolf
Am 25.10.2011 10:23, schrieb ronnie sahlberg: > Hi, > > > On Tue, Oct 25, 2011 at 7:17 PM, Kevin Wolf wrote: >> Am 25.10.2011 10:04, schrieb ronnie sahlberg: iscsi_destroy_url() is only ever called here. Do we leak it in the normal path? >>> >>> It didnt leak but the the label was con

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-25 Thread ronnie sahlberg
Hi, On Tue, Oct 25, 2011 at 7:17 PM, Kevin Wolf wrote: > Am 25.10.2011 10:04, schrieb ronnie sahlberg: >>> iscsi_destroy_url() is only ever called here. Do we leak it in the normal >>> path? >> >> It didnt leak but the the label was confusing. I have changed it from >> "failed" to "finished" si

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-25 Thread Kevin Wolf
Am 25.10.2011 10:04, schrieb ronnie sahlberg: >> iscsi_destroy_url() is only ever called here. Do we leak it in the normal >> path? > > It didnt leak but the the label was confusing. I have changed it from > "failed" to "finished" since this part > is executed for both normal and failures. But t

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-25 Thread ronnie sahlberg
Kevin, Thanks for the review, I have attached a modified patch that addresses all your comments. Please review and/or apply. > I doubt that you really need sysemu.h. If you did, I think you couldn't > successfully build qemu-img and qemu-io. You are right. I removed it. > Is acb->status == -E

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-24 Thread Kevin Wolf
Am 21.09.2011 11:37, schrieb Ronnie Sahlberg: > This provides built-in support for iSCSI to QEMU. > This has the advantage that the iSCSI devices need not be made visible to the > host, which is useful if you have very many virtual machines and very many > iscsi devices. > It also has the benefit

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread Daniel P. Berrange
On Thu, Oct 13, 2011 at 11:01:49AM +0100, Daniel P. Berrange wrote: > On Thu, Oct 13, 2011 at 08:46:54PM +1100, ronnie sahlberg wrote: > > Previous version of the patch received very positive feedback and > > several expressed seeing positive value of a built-in initiator. > > I updated patch from

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread Kevin Wolf
Am 13.10.2011 11:46, schrieb ronnie sahlberg: > Previous version of the patch received very positive feedback and > several expressed seeing positive value of a built-in initiator. > I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it. > > > Is there some other problem with th

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread Daniel P. Berrange
On Thu, Oct 13, 2011 at 08:46:54PM +1100, ronnie sahlberg wrote: > Previous version of the patch received very positive feedback and > several expressed seeing positive value of a built-in initiator. > I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it. > > > Is there some ot

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread Stefan Hajnoczi
On Thu, Oct 13, 2011 at 10:46 AM, ronnie sahlberg wrote: > Previous version of the patch received very positive feedback and > several expressed seeing positive value of a built-in initiator. > I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it. I'm happy. Stefan

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread Paolo Bonzini
On 10/13/2011 11:46 AM, ronnie sahlberg wrote: Previous version of the patch received very positive feedback and several expressed seeing positive value of a built-in initiator. I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it. Is there some other problem with the patch I

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-13 Thread ronnie sahlberg
Previous version of the patch received very positive feedback and several expressed seeing positive value of a built-in initiator. I updated patch from feedback 3 weeks ago and Stefan kindly reviewed it. Is there some other problem with the patch I am not aware of that I should address? I have b

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-10-09 Thread ronnie sahlberg
ping? On Thu, Sep 29, 2011 at 4:54 PM, Stefan Hajnoczi wrote: > On Wed, Sep 21, 2011 at 07:37:55PM +1000, Ronnie Sahlberg wrote: >> This provides built-in support for iSCSI to QEMU. >> This has the advantage that the iSCSI devices need not be made visible to >> the host, which is useful if you

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-29 Thread Stefan Hajnoczi
On Wed, Sep 21, 2011 at 07:37:55PM +1000, Ronnie Sahlberg wrote: > This provides built-in support for iSCSI to QEMU. > This has the advantage that the iSCSI devices need not be made visible to the > host, which is useful if you have very many virtual machines and very many > iscsi devices. > It a

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-27 Thread Paolo Bonzini
On 09/27/2011 10:08 PM, ronnie sahlberg wrote: List, What remains before this patch can be accepted? Previous patch received good feedback and severa people indicated that they would find the feature useful for several use cases. Kevin is on vacation this week. :) Paolo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-27 Thread ronnie sahlberg
List, What remains before this patch can be accepted? Previous patch received good feedback and severa people indicated that they would find the feature useful for several use cases. regards ronnie sahlberg On Wed, Sep 21, 2011 at 7:52 PM, ronnie sahlberg wrote: > On Wed, Sep 21, 2011 at 7:45 P

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-23 Thread Paolo Bonzini
On 09/23/2011 11:15 AM, Mark Wu wrote: I tested this patch with the following command: x86_64-softmmu/qemu-system-x86_64 --enable-kvm rhel54_1.img -m 1024 -net tap,ifname=tap0,script=no -net nic,model=virtio -sdl -drive file=iscsi://127.0.0.1/iqn.2011-09.com.example:server.target1/ And I found th

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-23 Thread Mark Wu
I tested this patch with the following command: x86_64-softmmu/qemu-system-x86_64 --enable-kvm rhel54_1.img -m 1024 -net tap,ifname=tap0,script=no -net nic,model=virtio -sdl -drive file=iscsi://127.0.0.1/iqn.2011-09.com.example:server.target1/ And I found that the whole qemu process would get fr

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-21 Thread ronnie sahlberg
On Wed, Sep 21, 2011 at 7:45 PM, Paolo Bonzini wrote: > On 09/21/2011 11:37 AM, Ronnie Sahlberg wrote: >> >> This driver interfaces with the multiplatform posix library for iscsi >> initiator/client access to iscsi devices hosted at >>     git://github.com/sahlberg/libiscsi.git > > Do you plan to

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-21 Thread ronnie sahlberg
Stefan, Thanks for your review. I feel that iscsi would be beneficial for several usecases. I have implemented all changes you pointed out, except two, and resent an updated patch to the list. Please see below. regards ronnie sahlberg On Mon, Sep 12, 2011 at 7:14 PM, Stefan Hajnoczi wrote: >

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-21 Thread Paolo Bonzini
On 09/21/2011 11:37 AM, Ronnie Sahlberg wrote: This driver interfaces with the multiplatform posix library for iscsi initiator/client access to iscsi devices hosted at git://github.com/sahlberg/libiscsi.git Do you plan to make a stable release, so that it can be packaged e.g. in Fedora?

[Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-21 Thread Ronnie Sahlberg
This provides built-in support for iSCSI to QEMU. This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices. It also has the benefit that non-root users of QEMU can access iSCSI devices a

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-17 Thread Stefan Hajnoczi
On Fri, Sep 16, 2011 at 05:53:20PM +0200, Christoph Hellwig wrote: > On Wed, Sep 14, 2011 at 04:50:25PM +0100, Stefan Hajnoczi wrote: > > I think in this case it will not make the code nicer. Since the > > external iSCSI library is based on callbacks it would be necessary to > > write the coroutin

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-16 Thread Christoph Hellwig
On Wed, Sep 14, 2011 at 04:50:25PM +0100, Stefan Hajnoczi wrote: > I think in this case it will not make the code nicer. Since the > external iSCSI library is based on callbacks it would be necessary to > write the coroutines<->callbacks adapter functions. So for example, > the READ10 command wou

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Orit Wasserman
On Thu, 2011-09-15 at 14:58 +0200, Paolo Bonzini wrote: > On 09/15/2011 02:34 PM, Orit Wasserman wrote: > >> > Then you need an iSCSI*target* that understands qcow2, like qemu-nbd > >> > but for iSCSI... that's exactly the thing you were worried about > >> > implementing. > > > > Not really , t

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Paolo Bonzini
On 09/15/2011 02:34 PM, Orit Wasserman wrote: > Then you need an iSCSI*target* that understands qcow2, like qemu-nbd > but for iSCSI... that's exactly the thing you were worried about > implementing. Not really , this is just part of the target configuration. For each file in the chain we c

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Orit Wasserman
On Thu, 2011-09-15 at 13:58 +0200, Paolo Bonzini wrote: > On 09/15/2011 01:42 PM, Dor Laor wrote: > >> > >> However, iSCSI is a complex protocol and a complex suite of tools. With > >> a simpler set of tools such as qemu-nbd/nbd, you can for example tunnel > >> data over the libvirt connection. Pos

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Paolo Bonzini
On 09/15/2011 02:01 PM, Dor Laor wrote: My main motivation for external iScsi is to provide qemu operations w/ non shared storage. iSCSI with multiple initiator is shared storage. Exactly :) that's the magic. I think we're on the same page, it's just a difference in terms. iSCSI or NBD wo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Dor Laor
On 09/15/2011 02:46 PM, Christoph Hellwig wrote: On Thu, Sep 15, 2011 at 02:42:42PM +0300, Dor Laor wrote: My main motivation for external iScsi is to provide qemu operations w/ non shared storage. iSCSI with multiple initiator is shared storage. Exactly :) that's the magic.

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Paolo Bonzini
On 09/15/2011 01:42 PM, Dor Laor wrote: However, iSCSI is a complex protocol and a complex suite of tools. With a simpler set of tools such as qemu-nbd/nbd, you can for example tunnel data over the libvirt connection. Possibly with encryption. Also, with iSCSI you're tied to raw, while qemu-nbd

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Christoph Hellwig
On Thu, Sep 15, 2011 at 02:42:42PM +0300, Dor Laor wrote: > My main motivation for external iScsi is to provide qemu operations w/ non > shared storage. iSCSI with multiple initiator is shared storage.

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Dor Laor
On 09/15/2011 12:11 PM, Paolo Bonzini wrote: On 09/15/2011 10:48 AM, Dor Laor wrote: We need the same patch for NBD, so I wouldn't bother with the synchronous flush. It seems to me that using a qemu external initiator/target pairs like Orit's original design in http://wiki.qemu.org/Features/Li

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread ronnie sahlberg
On Thu, Sep 15, 2011 at 7:11 PM, Paolo Bonzini wrote: ... > Perhaps Ronnie has rough performance numbers comparing in-kernel iSCSI with > libiscsi? I did some tests some time ago just doing things like 'dd if=/dev/sda of=/dev/null' on the root disk from within a RHEL guest itself and performace w

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Daniel P. Berrange
On Thu, Sep 15, 2011 at 11:48:35AM +0300, Dor Laor wrote: > On 09/15/2011 09:04 AM, Paolo Bonzini wrote: > >On 09/15/2011 01:08 AM, ronnie sahlberg wrote: > >>I think it is reasonable to just not support iscsi at all for > >>blocksize that is not multiple of 512 bytes > >>since a read-modify-write

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Paolo Bonzini
On 09/15/2011 11:10 AM, Kevin Wolf wrote: > We need the same patch for NBD, so I wouldn't bother with the > synchronous flush. Doesn't Stefan's patch conflict with your bdrv_co_flush patch? Yes. I'll include it myself in my NBD v2. Paolo

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Paolo Bonzini
On 09/15/2011 10:48 AM, Dor Laor wrote: We need the same patch for NBD, so I wouldn't bother with the synchronous flush. It seems to me that using a qemu external initiator/target pairs like Orit's original design in http://wiki.qemu.org/Features/LiveBlockMigration#ISCSI_for_non_shared_storage

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Kevin Wolf
Am 15.09.2011 08:04, schrieb Paolo Bonzini: > On 09/15/2011 01:08 AM, ronnie sahlberg wrote: >> I think it is reasonable to just not support iscsi at all for >> blocksize that is not multiple of 512 bytes >> since a read-modify-write cycle across a network would probably be >> prohibitively expensi

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Kevin Wolf
Am 15.09.2011 00:51, schrieb ronnie sahlberg: > On Thu, Sep 15, 2011 at 12:36 AM, Christoph Hellwig wrote: > ... +/* + * We support iscsi url's on the form + * iscsi://[%@][:]// + */ >> >> Is having username + password on the command line really a that good idea? >> Also what a

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Dor Laor
On 09/15/2011 09:04 AM, Paolo Bonzini wrote: On 09/15/2011 01:08 AM, ronnie sahlberg wrote: I think it is reasonable to just not support iscsi at all for blocksize that is not multiple of 512 bytes since a read-modify-write cycle across a network would probably be prohibitively expensive. Agre

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-15 Thread Daniel P. Berrange
On Thu, Sep 15, 2011 at 08:51:00AM +1000, ronnie sahlberg wrote: > On Thu, Sep 15, 2011 at 12:36 AM, Christoph Hellwig wrote: > ... > >> > +/* > >> > + * We support iscsi url's on the form > >> > + * iscsi://[%@][:]// > >> > + */ > > > > Is having username + password on the command line really a t

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-14 Thread Paolo Bonzini
On 09/15/2011 01:08 AM, ronnie sahlberg wrote: I think it is reasonable to just not support iscsi at all for blocksize that is not multiple of 512 bytes since a read-modify-write cycle across a network would probably be prohibitively expensive. Agreed. .bdrv_flush() I can easily add a synchro

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-14 Thread ronnie sahlberg
Stefan, Thanks for your review. I will do the changes you recommend and send an updated patch over the weekend. Could you advice the best path for handling the .bdrv_flush and the blocksize questions? I think it is reasonable to just not support iscsi at all for blocksize that is not multiple

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-14 Thread ronnie sahlberg
On Thu, Sep 15, 2011 at 12:36 AM, Christoph Hellwig wrote: ... >> > +/* >> > + * We support iscsi url's on the form >> > + * iscsi://[%@][:]// >> > + */ > > Is having username + password on the command line really a that good idea? > Also what about the more complicated iSCSI authentification sche

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-14 Thread Stefan Hajnoczi
On Wed, Sep 14, 2011 at 3:36 PM, Christoph Hellwig wrote: > On Mon, Sep 12, 2011 at 10:14:08AM +0100, Stefan Hajnoczi wrote: >> > +static void >> > +iscsi_aio_cancel(BlockDriverAIOCB *blockacb) >> > +{ >> > +    IscsiAIOCB *acb = (IscsiAIOCB *)blockacb; >> > +    IscsiLun *iscsilun = acb->iscsilun

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-14 Thread Christoph Hellwig
On Mon, Sep 12, 2011 at 10:14:08AM +0100, Stefan Hajnoczi wrote: > > +static void > > +iscsi_aio_cancel(BlockDriverAIOCB *blockacb) > > +{ > > +IscsiAIOCB *acb = (IscsiAIOCB *)blockacb; > > +IscsiLun *iscsilun = acb->iscsilun; > > + > > +acb->status = -ECANCELED; > > +acb->common.cb

Re: [Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-12 Thread Stefan Hajnoczi
On Sat, Sep 10, 2011 at 02:23:30PM +1000, Ronnie Sahlberg wrote: Looking good. I think this is worth merging because it does offer benefits over host iSCSI. > +static void > +iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void > *command_data, > +void *private_

[Qemu-devel] [PATCH] This patch adds a new block driver : iSCSI

2011-09-09 Thread Ronnie Sahlberg
This provides built-in support for iSCSI to QEMU. This has the advantage that the iSCSI devices need not be made visible to the host, which is useful if you have very many virtual machines and very many iscsi devices. It also has the benefit that non-root users of QEMU can access iSCSI devices a