Re: [PATCH v2 5/6] tests: unit tests for iSCSI iSER

On Fri, May 15, 2020 at 09:03:39 +0100, Daniel Berrange wrote: > On Fri, May 15, 2020 at 11:46:04AM +0800, Han Han wrote: > > Signed-off-by: Han Han <hhan(a)redhat.com&gt; > > --- > > .../qemuxml2argvdata/disk-network-iscsi.args | 8 +++- > > .../disk-network-iscsi.x86_64-2.12.0.args | 7 ++- > > .../disk-network-iscsi.x86_64-latest.args | 45 +++++++++++-------- > > tests/qemuxml2argvdata/disk-network-iscsi.xml | 9 ++++ > > tests/qemuxml2argvtest.c | 5 ++- > > .../qemuxml2xmloutdata/disk-network-iscsi.xml | 10 +++++ > > tests/qemuxml2xmltest.c | 4 +- > > tests/virstoragetest.c | 16 +++++++ > > 8 files changed, 80 insertions(+), 24 deletions(-) > > diff --git a/tests/qemuxml2argvdata/disk-network-iscsi.xml b/tests/qemuxml2argvdata/disk-network-iscsi.xml > > index 8a55f1b2..9812b4b1 100644 > > --- a/tests/qemuxml2argvdata/disk-network-iscsi.xml > > +++ b/tests/qemuxml2argvdata/disk-network-iscsi.xml > > @@ -55,6 +56,14 @@ > > </source> > > <target dev='sda' bus='scsi'/> > > </disk> > > + <disk type='network' device='lun'> > > + <driver name='qemu' type='raw'/> > > + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> > > + <host name='example.org'/> > > + <iser/> > > + </source> > > + <target dev='sdb' bus='scsi'/> > > + </disk> > > I'm thinking would be better to have this represented as > an attribute for the host, as we're basically choosing between > TCP and RMDA as the network link, over which ISCSI is run eg > > <source protocol='iscsi' name='iqn.1992-01.com.example/1'> > <host name='example.org' transport="tcp|rdma"/> > </source> I specifically rejected this in the last review. The issue is that 'rdma' is a wrong term. It's not even used by qemu any more. Additionally iser runs on top of a variety of transport depending on what technology you have so encoding it as a transport is wrong. See https://www.redhat.com/archives/libvir-list/2020-May/msg00397.html

> If we really want to keep the "iser" terminology, then it would > be as a replacement for protocol="iscsi" instead g > > <source protocol='iser' name='iqn.1992-01.com.example/1'> Yes. IMO this is the only way to go. Specifically it will also make other things like disk type='volume' les weird IMO.