<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Mar 14, 2018 at 9:04 PM Richard W.M. Jones <<a href="mailto:rjones@redhat.com">rjones@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Mar 14, 2018 at 06:56:19PM +0000, Nir Soffer wrote:<br>
> I posted documentation for the new API optimized for random I/O:<br>
> <a href="https://gerrit.ovirt.org/#/c/89022/" rel="noreferrer" target="_blank">https://gerrit.ovirt.org/#/c/89022/</a><br>
<br>
Wish I'd had this documentation when I started the patch :-)<br>
Yes, it's much clearer.<br>
<br>
> I changed POST to PATCH to match the existing /tickets API, and<br>
> this also seems to be more standard way to do such operations.<br>
<br>
Assuming Python httplib will allow us to put anything in the method<br>
argument of http.putrequest then this doesn't appear to make any<br>
significant difference so that's fine. Also we can set the "flush"<br>
(ie. FUA) parameter to match the NBD request.<br>
<br>
> Please check and comment if this makes sense and serves the v2v<br>
> use case or other uses case we missed.<br>
><br>
> I think we can implement all of this for 4.2.4, but:<br>
><br>
> - using simple zero loop, as in <a href="https://gerrit.ovirt.org/#/c/88793/" rel="noreferrer" target="_blank">https://gerrit.ovirt.org/#/c/88793/</a>.<br>
> later we can make it more efficient.<br>
> - trim is a noop, maybe we will be able to support it in 4.3<br>
> - flush - may be noop now (all requests will implicitly flush).<br>
<br>
I don't think we really need trim or flush. They're only minor<br>
optimizations. Zero is the one which is required.<br>
<br>
FWIW NBD allows you to flush ranges or flush the whole disk, in case<br>
that matters (your proposal only allows you to flush the whole disk).<br></blockquote><div><br></div><div>What is the use case for flushing ranges? I guess we will have one</div><div>or few flushes per images.</div><div><br></div><div>Looking at sync_file_range(2), it does not seem to be a safe way to </div><div>flush:</div><div><br></div><div><div> Warning</div><div><br></div><div> This system call is extremely dangerous and should not be used in</div><div> portable programs. None of these operations writes out the file's</div><div> metadata. Therefore, unless the application is strictly performing</div><div> overwrites of already-instantiated disk blocks, there are no guarantees</div><div> that the data will be available after a crash. There is no user</div><div> interface to know if a write is purely an overwrite. On file systems</div><div> using copy-on-write semantics (e.g., btrfs) an overwrite of existing</div><div> allocated blocks is impossible. When writing into preal‐ located</div><div> space, many file systems also require calls into the block allocator,</div><div> which this system call does not sync out to disk. This system call</div><div> does not flush disk write caches and thus does not provide any data</div><div> integrity on systems with volatile disk write caches. <br><br></div></div><div>I can support the same size and offset arguments, and treat them</div><div>as a hint if we can implement this safely in some future version.</div><div>But I think providing only simple and safe way to flush is good</div><div>enough for this context.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> I think we better have complete API with partial or simpler<br>
> implementation now, to minimize the hacks needed in v2v and<br>
> other clients.<br>
<br>
Agreed.<br>
<br>
Rich.<br>
<br>
--<br>
Richard Jones, Virtualization Group, Red Hat <a href="http://people.redhat.com/~rjones" rel="noreferrer" target="_blank">http://people.redhat.com/~rjones</a><br>
Read my programming and virtualization blog: <a href="http://rwmj.wordpress.com" rel="noreferrer" target="_blank">http://rwmj.wordpress.com</a><br>
virt-p2v converts physical machines to virtual machines. Boot with a<br>
live CD or over the network (PXE) and turn machines into KVM guests.<br>
<a href="http://libguestfs.org/virt-v2v" rel="noreferrer" target="_blank">http://libguestfs.org/virt-v2v</a><br>
</blockquote></div></div>