From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X9gKN-0006Lp-Kp for qemu-devel@nongnu.org; Tue, 22 Jul 2014 16:06:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X9gKC-0005zf-Uk for qemu-devel@nongnu.org; Tue, 22 Jul 2014 16:06:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48034) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X9gKC-0005zV-MY for qemu-devel@nongnu.org; Tue, 22 Jul 2014 16:06:12 -0400 Message-ID: <53CEC441.5020801@redhat.com> Date: Tue, 22 Jul 2014 22:06:25 +0200 From: Max Reitz MIME-Version: 1.0 References: <1405802159-2355-1-git-send-email-mreitz@redhat.com> <1405802159-2355-2-git-send-email-mreitz@redhat.com> <53CD3728.1040904@redhat.com> In-Reply-To: <53CD3728.1040904@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-img: Allow source cache mode specification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Lieven , Stefan Hajnoczi On 21.07.2014 17:52, Eric Blake wrote: > On 07/19/2014 02:35 PM, Max Reitz wrote: >> Many qemu-img subcommands only read the source file(s) once. For these >> use cases, a full write-back cache is unnecessary and mainly clutters >> host cache memory. Though this is generally no concern as cache memory >> is freely available and can be scaled by the host OS, it may become a >> concern with thin provisioning. >> >> For these cases, it makes sense to allow users to freely specify the >> source cache mode (e.g. use no cache at all). >> >> This commit adds a new switch (-T) for the qemu-img subcommands check, >> compare, convert and rebase to specify the cache to be used for source >> images (the backing file in case of rebase). > What mnemonic did you have in mind when choosing -T? Or was it just a > universally available letter for the subcommands you were touching? To be honest, I just didn't know what -t stands for. Therefore I just thought it might be remotely logical if the lower-cased letter is used for destination and the upper-cased letter for source caching. >> Signed-off-by: Max Reitz >> --- >> qemu-img-cmds.hx | 16 ++++++------ >> qemu-img.c | 78 ++++++++++++++++++++++++++++++++++++++++++++------------ >> qemu-img.texi | 14 +++++++--- >> 3 files changed, 80 insertions(+), 28 deletions(-) >> >> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx >> index d029609..5613628 100644 >> --- a/qemu-img-cmds.hx >> +++ b/qemu-img-cmds.hx >> @@ -10,9 +10,9 @@ STEXI >> ETEXI >> >> DEF("check", img_check, >> - "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] filename") >> + "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename") > Might be nice to fix the unintentional double space before -r while > touching this line. Okay, I'll send a v2 (taking into account your comments for patch 2). >> DEF("convert", img_convert, >> - "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename") >> + "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename") > Oh, maybe you just picked -T for source because -t was already picked > for destination? Right. :-) > At any rate, seems reasonable. > Reviewed-by: Eric Blake Thanks, Max