Re: VMD: vioscsi - fix large iso support
On Fri, Jan 19, 2018 at 07:35:08PM +1000, David Gwynne wrote:
>
>
> > On 19 Jan 2018, at 4:32 pm, Carlos Cardenas wrote:
> >
> > Howdy.
> >
> > Attached is a patch to fix handling images > 4GB support in Linux. I
> > confused image size vs blocks to determine when to send UINT32_MAX
> > and trigger a READ_CAPACITY_16.
> >
> > Identified by mlarkin@
> >
> > Comments? Ok?
>
> ok.
>
> after this you should make another fix though: READ CAPACITY 10 and 16 should
> return the last addressable block, not than the number of blocks.
>
> in other words you need to take 1 from n_blocks before processing it for
> scsi. a good exapmle of this is
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c#rev1.55 and
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c.diff?r1=1.54&r2=1.55
>
> dlg
That's already handled in READ_CAPACITY_* and GET_CONFIGURATION,
i.e. _lto4b(dev->n_blocks - 1, r_cap_data->length);
Along with checks in READ_* to ensure the requested lba is < n_blocks - 1.
Yes...I was bitten by that earlier on in the development of the driver. :-|
+--+
Carlos
>
> >
> > +--+
> > Carlos
> > Index: vioscsi.c
> > ===
> > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
> > retrieving revision 1.3
> > diff -u -p -a -u -r1.3 vioscsi.c
> > --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
> > +++ vioscsi.c 19 Jan 2018 06:26:14 -
> > @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
> > __func__, dev->sz, dev->n_blocks);
> >
> > /*
> > -* determine if size of iso image > UINT32_MAX
> > +* determine if num blocks of iso image > UINT32_MAX
> > * if it is, set addr to UINT32_MAX (0x)
> > * indicating to hosts that READ_CAPACITY_16 should
> > * be called to retrieve the full size
> > */
> > - if (dev->sz >= UINT32_MAX) {
> > + if (dev->n_blocks >= UINT32_MAX) {
> > _lto4b(UINT32_MAX, r_cap_data->addr);
> > _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
> > log_warnx("%s: ISO sz %lld is bigger than "
> > @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
> > config_random_read_desc->feature_code);
> > config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
> > config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
> > - if (dev->sz >= UINT32_MAX)
> > + if (dev->n_blocks >= UINT32_MAX)
> > _lto4b(UINT32_MAX, config_random_read_desc->block_size);
> > else
> > _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
>
Re: VMD: vioscsi - fix large iso support
> On 19 Jan 2018, at 4:32 pm, Carlos Cardenas wrote:
>
> Howdy.
>
> Attached is a patch to fix handling images > 4GB support in Linux. I
> confused image size vs blocks to determine when to send UINT32_MAX
> and trigger a READ_CAPACITY_16.
>
> Identified by mlarkin@
>
> Comments? Ok?
ok.
after this you should make another fix though: READ CAPACITY 10 and 16 should
return the last addressable block, not than the number of blocks.
in other words you need to take 1 from n_blocks before processing it for scsi.
a good exapmle of this is
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c#rev1.55 and
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c.diff?r1=1.54&r2=1.55
dlg
>
> +--+
> Carlos
> Index: vioscsi.c
> ===
> RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
> retrieving revision 1.3
> diff -u -p -a -u -r1.3 vioscsi.c
> --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
> +++ vioscsi.c 19 Jan 2018 06:26:14 -
> @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
> __func__, dev->sz, dev->n_blocks);
>
> /*
> - * determine if size of iso image > UINT32_MAX
> + * determine if num blocks of iso image > UINT32_MAX
>* if it is, set addr to UINT32_MAX (0x)
>* indicating to hosts that READ_CAPACITY_16 should
>* be called to retrieve the full size
>*/
> - if (dev->sz >= UINT32_MAX) {
> + if (dev->n_blocks >= UINT32_MAX) {
> _lto4b(UINT32_MAX, r_cap_data->addr);
> _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
> log_warnx("%s: ISO sz %lld is bigger than "
> @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
> config_random_read_desc->feature_code);
> config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
> config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
> - if (dev->sz >= UINT32_MAX)
> + if (dev->n_blocks >= UINT32_MAX)
> _lto4b(UINT32_MAX, config_random_read_desc->block_size);
> else
> _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
Re: VMD: vioscsi - fix large iso support
On Thu, Jan 18, 2018 at 10:32:31PM -0800, Carlos Cardenas wrote:
> Howdy.
>
> Attached is a patch to fix handling images > 4GB support in Linux. I
> confused image size vs blocks to determine when to send UINT32_MAX
> and trigger a READ_CAPACITY_16.
>
> Identified by mlarkin@
>
> Comments? Ok?
>
> +--+
> Carlos
ok mlarkin
> Index: vioscsi.c
> ===
> RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
> retrieving revision 1.3
> diff -u -p -a -u -r1.3 vioscsi.c
> --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
> +++ vioscsi.c 19 Jan 2018 06:26:14 -
> @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
> __func__, dev->sz, dev->n_blocks);
>
> /*
> - * determine if size of iso image > UINT32_MAX
> + * determine if num blocks of iso image > UINT32_MAX
>* if it is, set addr to UINT32_MAX (0x)
>* indicating to hosts that READ_CAPACITY_16 should
>* be called to retrieve the full size
>*/
> - if (dev->sz >= UINT32_MAX) {
> + if (dev->n_blocks >= UINT32_MAX) {
> _lto4b(UINT32_MAX, r_cap_data->addr);
> _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
> log_warnx("%s: ISO sz %lld is bigger than "
> @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
> config_random_read_desc->feature_code);
> config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
> config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
> - if (dev->sz >= UINT32_MAX)
> + if (dev->n_blocks >= UINT32_MAX)
> _lto4b(UINT32_MAX, config_random_read_desc->block_size);
> else
> _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
VMD: vioscsi - fix large iso support
Howdy.
Attached is a patch to fix handling images > 4GB support in Linux. I
confused image size vs blocks to determine when to send UINT32_MAX
and trigger a READ_CAPACITY_16.
Identified by mlarkin@
Comments? Ok?
+--+
Carlos
Index: vioscsi.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
retrieving revision 1.3
diff -u -p -a -u -r1.3 vioscsi.c
--- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
+++ vioscsi.c 19 Jan 2018 06:26:14 -
@@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
__func__, dev->sz, dev->n_blocks);
/*
-* determine if size of iso image > UINT32_MAX
+* determine if num blocks of iso image > UINT32_MAX
* if it is, set addr to UINT32_MAX (0x)
* indicating to hosts that READ_CAPACITY_16 should
* be called to retrieve the full size
*/
- if (dev->sz >= UINT32_MAX) {
+ if (dev->n_blocks >= UINT32_MAX) {
_lto4b(UINT32_MAX, r_cap_data->addr);
_lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
log_warnx("%s: ISO sz %lld is bigger than "
@@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
config_random_read_desc->feature_code);
config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
- if (dev->sz >= UINT32_MAX)
+ if (dev->n_blocks >= UINT32_MAX)
_lto4b(UINT32_MAX, config_random_read_desc->block_size);
else
_lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
