Re: [Qemu-block] [Nbd] write_zeroes/trim on the whole disk

[Wouter Verhelst]

On Sat, Sep 24, 2016 at 11:31:25AM +0100, Alex Bligh wrote:
> 
> > On 23 Sep 2016, at 22:21, Wouter Verhelst  wrote:
> > 
> > On Fri, Sep 23, 2016 at 02:00:06PM -0500, Eric Blake wrote:
> >> My preference would be a new flag to the existing commands, with
> >> explicit documentation that 0 offset and 0 length must be used with that
> >> flag, when requesting a full-device wipe.
> > 
> > Alternatively, what about a flag that says "if you use this flag, the
> > size should be left-shifted by X bits before processing"? That allows
> > you to do TRIM or WRITE_ZEROES on much larger chunks, without being
> > limited to "whole disk" commands. We should probably make it an illegal
> > flag for any command that actually sends data over the wire, though.
> 
> I'd prefer an approach like this. Perhaps X could be negotiated
> with the block size extension (or simply be defined as the
> 'preferred block size'.
> 
> This could then be defined to work with all commands.

The downside of making the shift size too small is that it wouldn't help
as much with the use case that was presented here (that is, blanking a
whole disk with one command). I think if the shift size is to be
negotiated, it should be something that the client prefers, rather than
something the server prefers.

I really don't think allowing a shift for things that requires data
transfer makes much sense. Yes, a shift for things like "clear all data"
can be sensible, because (depending on backend) this can be a fairly
simple operation. However, "write more than 2G of data" or "read more
than 2G of data" isn't a simple operation and takes a lot of data to be
transferred, so allowing shift for those operations doesn't make sense,
at all; and suggesting that we allow it could actually make people
believe it's a good idea to do so when it really really (really) isn't.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

Re: [Qemu-block] [Nbd] write_zeroes/trim on the whole disk

[Carl-Daniel Hailfinger]

On 24.09.2016 19:33, Vladimir Sementsov-Ogievskiy wrote:
> On 24.09.2016 20:13, Vladimir Sementsov-Ogievskiy wrote:
>> I agree that requests larger than disk size are ugly.. But splitting
>> request brings me again to idea of having separate command or flag
>> for clearing the whole disk without that dance. Server may report
>> availability of this/flag command only if target driver supports fast
>> write_zeroes (qcow2 in our case). 
> Also, such flag may be used to satisfy all needs:
>
> flag BIG_REQUEST is set and length = 0->request on the whole 
> disk, offset must be 0
> flag BIG_REQUEST is set and length > 0->request on 
> (offset*block_size, length*block_size), length*block_size must be <= 
> disk_size

What happens if length*block_size<=disk_size, but
offset*block_size+length*block_size>disk_size? Wraparound?

Regards,
Carl-Daniel

Re: [Qemu-block] [Nbd] write_zeroes/trim on the whole disk

[Vladimir Sementsov-Ogievskiy]

On 24.09.2016 23:14, Carl-Daniel Hailfinger wrote:

On 24.09.2016 19:33, Vladimir Sementsov-Ogievskiy wrote:

On 24.09.2016 20:13, Vladimir Sementsov-Ogievskiy wrote:

I agree that requests larger than disk size are ugly.. But splitting
request brings me again to idea of having separate command or flag
for clearing the whole disk without that dance. Server may report
availability of this/flag command only if target driver supports fast
write_zeroes (qcow2 in our case).

Also, such flag may be used to satisfy all needs:

flag BIG_REQUEST is set and length = 0->request on the whole
disk, offset must be 0
flag BIG_REQUEST is set and length > 0->request on
(offset*block_size, length*block_size), length*block_size must be <=
disk_size

What happens if length*block_size<=disk_size, but
offset*block_size+length*block_size>disk_size? Wraparound?


Personally, for me main scenario for is length=0, to clear the whole 
disk. In general allowing requests larger than disk size may not be very 
good idea.. Also:


On 24.09.2016 19:49, Alex Bligh wrote:

>May be, additional option, specifying the shift would be better. With 
convention that if offset+length exceeds disk size, length should be recalculated 
as disk_size-offset.

I don't think we should do that. We already have clear semantics that prevent 
operations beyond the end of the disk. Again, just break the command up into 
multipl commands. No great hardship.


So, most possible answer on your question: it should be an error.



Regards,
Carl-Daniel



--
Best regards,
Vladimir

Re: [Qemu-block] [Nbd] write_zeroes/trim on the whole disk

[Alex Bligh]

> On 23 Sep 2016, at 22:21, Wouter Verhelst  wrote:
> 
> On Fri, Sep 23, 2016 at 02:00:06PM -0500, Eric Blake wrote:
>> My preference would be a new flag to the existing commands, with
>> explicit documentation that 0 offset and 0 length must be used with that
>> flag, when requesting a full-device wipe.
> 
> Alternatively, what about a flag that says "if you use this flag, the
> size should be left-shifted by X bits before processing"? That allows
> you to do TRIM or WRITE_ZEROES on much larger chunks, without being
> limited to "whole disk" commands. We should probably make it an illegal
> flag for any command that actually sends data over the wire, though.

I'd prefer an approach like this. Perhaps X could be negotiated
with the block size extension (or simply be defined as the
'preferred block size'.

This could then be defined to work with all commands.

-- 
Alex Bligh