[Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Andrey Korolyov]

Hello,

As we will never pass LUN#0 as a storage lun, it would be better to
prohibit this at least in iscsi.c, otherwise it will result in an FPU
exception and emulator crash:

traps: qemu-system-x86[32430] trap divide error ip:7f1dab7b5073
sp:7f1d713e4ae0 error:0 in block-iscsi.so[7f1dab7b0000+8000]

    353 static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
    354                                       IscsiLun *iscsilun)
    355 {
    356     if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
    357         (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {

As far as I can see the LUN#0 can be thrown out on a top level, as one
will never use it directly as an iSCSI backend. Please correct me if
I`m wrong in this assumption.

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Peter Lieven]

Am 02.08.2015 um 13:42 schrieb Andrey Korolyov:
> Hello,
>
> As we will never pass LUN#0 as a storage lun, it would be better to
> prohibit this at least in iscsi.c, otherwise it will result in an FPU
> exception and emulator crash:
>
> traps: qemu-system-x86[32430] trap divide error ip:7f1dab7b5073
> sp:7f1d713e4ae0 error:0 in block-iscsi.so[7f1dab7b0000+8000]
>
>      353 static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
>      354                                       IscsiLun *iscsilun)
>      355 {
>      356     if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
>      357         (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {
>
> As far as I can see the LUN#0 can be thrown out on a top level, as one
> will never use it directly as an iSCSI backend. Please correct me if
> I`m wrong in this assumption.

Hi Andrey,

LUN 0 is quite common on iSCSI targets. I think what causes the problem
is not the LUN ID, but the target which returns 0 for the blocksize. Which
target are you using?

Peter

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Andrey Korolyov]

On Mon, Aug 3, 2015 at 9:45 AM, Peter Lieven <pl@kamp.de> wrote:
> Am 02.08.2015 um 13:42 schrieb Andrey Korolyov:
>>
>> Hello,
>>
>> As we will never pass LUN#0 as a storage lun, it would be better to
>> prohibit this at least in iscsi.c, otherwise it will result in an FPU
>> exception and emulator crash:
>>
>> traps: qemu-system-x86[32430] trap divide error ip:7f1dab7b5073
>> sp:7f1d713e4ae0 error:0 in block-iscsi.so[7f1dab7b0000+8000]
>>
>>      353 static bool is_request_lun_aligned(int64_t sector_num, int
>> nb_sectors,
>>      354                                       IscsiLun *iscsilun)
>>      355 {
>>      356     if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
>>      357         (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {
>>
>> As far as I can see the LUN#0 can be thrown out on a top level, as one
>> will never use it directly as an iSCSI backend. Please correct me if
>> I`m wrong in this assumption.
>
>
> Hi Andrey,
>
> LUN 0 is quite common on iSCSI targets. I think what causes the problem
> is not the LUN ID, but the target which returns 0 for the blocksize. Which
> target are you using?
>
> Peter

Hi Peter,

I`ve mistyped lun for tgtd upon volume hotplug, which resulted in an
accidental crash, there is nothing but human factor. Until only LUN0
may possess such unusual properties, I`d vote to explicitly work it
around instead of adding generic protection from volumes with
advertized zero block size.

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Peter Lieven]

Am 03.08.2015 um 09:47 schrieb Andrey Korolyov:
> On Mon, Aug 3, 2015 at 9:45 AM, Peter Lieven <pl@kamp.de> wrote:
>> Am 02.08.2015 um 13:42 schrieb Andrey Korolyov:
>>> Hello,
>>>
>>> As we will never pass LUN#0 as a storage lun, it would be better to
>>> prohibit this at least in iscsi.c, otherwise it will result in an FPU
>>> exception and emulator crash:
>>>
>>> traps: qemu-system-x86[32430] trap divide error ip:7f1dab7b5073
>>> sp:7f1d713e4ae0 error:0 in block-iscsi.so[7f1dab7b0000+8000]
>>>
>>>       353 static bool is_request_lun_aligned(int64_t sector_num, int
>>> nb_sectors,
>>>       354                                       IscsiLun *iscsilun)
>>>       355 {
>>>       356     if ((sector_num * BDRV_SECTOR_SIZE) % iscsilun->block_size ||
>>>       357         (nb_sectors * BDRV_SECTOR_SIZE) % iscsilun->block_size) {
>>>
>>> As far as I can see the LUN#0 can be thrown out on a top level, as one
>>> will never use it directly as an iSCSI backend. Please correct me if
>>> I`m wrong in this assumption.
>>
>> Hi Andrey,
>>
>> LUN 0 is quite common on iSCSI targets. I think what causes the problem
>> is not the LUN ID, but the target which returns 0 for the blocksize. Which
>> target are you using?
>>
>> Peter
> Hi Peter,
>
> I`ve mistyped lun for tgtd upon volume hotplug, which resulted in an
> accidental crash, there is nothing but human factor. Until only LUN0
> may possess such unusual properties, I`d vote to explicitly work it
> around instead of adding generic protection from volumes with
> advertized zero block size.

I will work out a patch that forbids to mount a LUN with zero blocksize.

Peter

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Paolo Bonzini]

On 03/08/2015 09:47, Andrey Korolyov wrote:
> I`ve mistyped lun for tgtd upon volume hotplug, which resulted in an
> accidental crash, there is nothing but human factor. Until only LUN0
> may possess such unusual properties, I`d vote to explicitly work it
> around instead of adding generic protection from volumes with
> advertized zero block size.

It's only tgtd, as far as I know, that makes LUN0 special.  With lio
(in-kernel target), for example, you can associate any kind of LUN to
any LUN number.

Paolo

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Peter Lieven]

Am 03.08.2015 um 10:13 schrieb Paolo Bonzini:
>
> On 03/08/2015 09:47, Andrey Korolyov wrote:
>> I`ve mistyped lun for tgtd upon volume hotplug, which resulted in an
>> accidental crash, there is nothing but human factor. Until only LUN0
>> may possess such unusual properties, I`d vote to explicitly work it
>> around instead of adding generic protection from volumes with
>> advertized zero block size.
> It's only tgtd, as far as I know, that makes LUN0 special.  With lio
> (in-kernel target), for example, you can associate any kind of LUN to
> any LUN number.

I would error out in iscsi_readcapacity_sync if the block_size is
not dividable by BDRV_SECTOR_SIZE or 0. Ok?

Peter

Re: [Qemu-devel] Forbid to pass lun 0 to iscsi driver

[Andrey Korolyov]

On Mon, Aug 3, 2015 at 11:13 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 03/08/2015 09:47, Andrey Korolyov wrote:
>> I`ve mistyped lun for tgtd upon volume hotplug, which resulted in an
>> accidental crash, there is nothing but human factor. Until only LUN0
>> may possess such unusual properties, I`d vote to explicitly work it
>> around instead of adding generic protection from volumes with
>> advertized zero block size.
>
> It's only tgtd, as far as I know, that makes LUN0 special.  With lio
> (in-kernel target), for example, you can associate any kind of LUN to
> any LUN number.
>
> Paolo

It`s quite controversial point, for example take a look on
https://lkml.org/lkml/2012/11/27/330. Anyway, if the lio does provide
storage on LUN0, my point from above is no longer valid, because I had
no references from quick googling confirming such a possibility.