<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 17, 2016 at 10:00 PM, Richard W.M. Jones <span dir="ltr">&lt;<a href="mailto:rjones@redhat.com" target="_blank">rjones@redhat.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, May 17, 2016 at 09:50:07PM +0300, Nir Soffer wrote:<br>
&gt; On Tue, May 17, 2016 at 7:14 PM, Shmuel Melamud &lt;<a href="mailto:smelamud@redhat.com">smelamud@redhat.com</a>&gt; wrote:<br>
&gt;<br>
&gt; &gt; Hi!<br>
&gt; &gt;<br>
&gt; &gt; There is an RFE being implemented currently (<br>
&gt; &gt; <a href="https://bugzilla.redhat.com/show_bug.cgi?id=734120" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=734120</a>) to use --inplace<br>
&gt; &gt; option in virt-sparsify to sparsify a disk image in-place, without copying<br>
&gt; &gt; it.<br>
&gt; &gt;<br>
&gt; &gt; The problem is that in-place sparsify works on NFS only starting from NFS<br>
&gt; &gt; 4.2, while the copying implementation supposedly works with any storage.<br>
&gt; &gt;<br>
&gt; &gt; From my point of view, it is better to remove the old code and start with<br>
&gt; &gt; the new code that uses --inplace and just add to document that one will<br>
&gt; &gt; need NFS &gt;= 4.2 for sparsify feature to work. Although the old<br>
&gt; &gt; implementation exists, it is not used currently so there are no actual<br>
&gt; &gt; users that may be affected by the change. If it will be a crying need to<br>
&gt; &gt; use sparsify on older NFS or some other incompatible storage, we can add it<br>
&gt; &gt; later on the base of the new code. The old code is far from ideal and we<br>
&gt; &gt; will need to rewrite it in any case, and there is not sense to do this<br>
&gt; &gt; without real need.<br>
&gt; &gt;<br>
&gt;<br>
&gt; Richard,<br>
&gt;<br>
&gt; Currently vdsm is using:<br>
&gt;<br>
&gt;     virt-sparsify --tmp prebuilt:tmp_vol --format src_format --convert<br>
&gt; dst_format src_vol dst_vol<br>
<br>
&gt; (--format and --dst_format are optional)<br>
&gt;<br>
&gt; tmp_vol, src_vol, and dst_vol can be either file on nfs/glusterfs/other<br>
&gt; posix shared filesystem,<br>
&gt; or an lv created on top of lun (iscsi/fc).<br>
&gt;<br>
&gt; can you confirm this method works on all the storage types I mentioned? or<br>
&gt; this depends<br>
&gt; on the underlying storage server?<br>
<br>
</div></div>Copying mode requires that the dst_vol supports sparseness.  The most<br>
common case where this would *not* be true is partitions on local hard<br>
disks.  You can&#39;t make a partition and/or local hard disk sparse.<br>
<br>
However all of the ones you&#39;ve mentioned above support sparseness, so<br>
you should be good.<br>
<br>
*If* you were using --inplace only, you could nuke the --tmp option,<br>
and indeed all the code associated with &quot;prebuilt&quot; qcow2 files for<br>
scratch space.<br>
<br>
That&#39;s because --inplace mode uses hardly any temporary storage, so<br>
you can just have it use regular /var/tmp or $TMPDIR.<br>
<br>
However ...<br>
<span class=""><br>
&gt; The new inplace method is much nicer, but something that works only on<br>
&gt; latest NFS version<br>
&gt; is not useful for our most important users, using block storage.<br>
<br>
</span>... it&#39;s a shame, but yes.<br>
<br>
Rich.<br></blockquote><div><br></div><div>Thanks Richard!</div><div><br></div><div>So we have existing code supporting al storage types, and can benefit</div><div>our most important users.</div><div><br></div><div>We can use the new much simpler method if a user is using NFS 4.2</div><div>but it is not a replacement.</div><div><br></div><div>So I would keep the current implementation in vdsm, and add a new much </div><div>simpler implementation for inplace sparsify.</div><div><br></div><div>We should implement the missing part on the engine side, managing </div><div>the entire flow.</div><div><br></div><div>Thoughts?</div><div><br></div><div>Nir</div><div><br></div></div></div></div>