On Wed, Dec 02, 2015 at 03:22:38PM -0500, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1270709 > > When a volume wipe is successful, a volume refresh should be done afterwards > to update any volume data that may be used in future volume commands, such as > volume resize. For a raw file volume, a wipe would truncate the file and > a followup volume resize the capacity may fail because the volume target > allocation isn't updated to reflect the wipe activity. > I would expect that after wiping a 200 MB volume with zeros, it would contain 200 MB of zeros and it would not be shrinkable to 50 MB, not even after a volume refresh. While it seems ftruncate to 0 bytes and back satisfies the documentation of the virStorageVolWipe API: Ensure data previously on a volume is not accessible to future reads the ALG_ZERO description says: VIR_STORAGE_VOL_WIPE_ALG_ZERO = 0 1-pass, all zeroes Do we need to update it to reflect that there might not be any pass over the old data (which might not happen for non-sparse files either, if the filesystem does not overwrite the same sectors)? > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > src/storage/storage_driver.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index bbf21f6..2e59e39 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -2436,7 +2436,19 @@ storageVolWipePattern(virStorageVolPtr obj, > goto cleanup; > } > > - ret = backend->wipeVol(obj->conn, pool, vol, algorithm, flags); > + if ((ret = backend->wipeVol(obj->conn, pool, vol, algorithm, flags)) < 0) > + goto cleanup; More readable as: if (func() < 0) goto cleanup; ret = 0; If the return value does not need to be propagated. (Which it should not here, but it is possible in some cases. I will send a patch) > + > + /* Best effort to refresh the volume data. If unsuccessful, we've already > + * wiped the data so there's no going back on that. Best we can do is > + * provide some details over what happened and move on > + */ The wipe did happen, but if refreshVol fails, there is something seriously wrong with the volume. I think returning -1 and reporting an error is reasonable here. Jan > + if (backend->refreshVol && > + backend->refreshVol(obj->conn, pool, vol) < 0) { > + VIR_WARN("failed to refresh volume '%s' info after volume wipe", > + vol->name); > + virResetLastError(); > + }
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list