From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MNm9H-000141-TD for qemu-devel@nongnu.org; Mon, 06 Jul 2009 07:14:15 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MNm9D-0000yl-Px for qemu-devel@nongnu.org; Mon, 06 Jul 2009 07:14:15 -0400 Received: from [199.232.76.173] (port=60233 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MNm9D-0000ye-OI for qemu-devel@nongnu.org; Mon, 06 Jul 2009 07:14:11 -0400 Received: from verein.lst.de ([213.95.11.210]:44667) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60) (envelope-from ) id 1MNm9D-0008Kh-7X for qemu-devel@nongnu.org; Mon, 06 Jul 2009 07:14:11 -0400 Date: Mon, 6 Jul 2009 13:14:08 +0200 From: Christoph Hellwig Subject: Re: [Qemu-devel] [PATCH] qemu-io: add flag to mark files growable Message-ID: <20090706111408.GA10163@lst.de> References: <20090704155330.GA4825@lst.de> <4A51BBA1.4090307@redhat.com> <20090706102023.GA7835@lst.de> <4A51D486.10005@redhat.com> <20090706105111.GA9469@lst.de> <4A51DA57.3010600@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A51DA57.3010600@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Christoph Hellwig , qemu-devel@nongnu.org On Mon, Jul 06, 2009 at 01:04:55PM +0200, Kevin Wolf wrote: > > Well, the primary use of qemu-iotests is to try out the I/O patterns, > > and the -g option is to check out we do the correct thing for growable > > files, so I'd really prefer it to fail if we're not allow to actually > > make it growable. > > Perfectly reasonable. But then make it really fail. Oh, now I see your complaint. The lack of a return 1 after the error message was not intentional - I've added it now. > > Now one think I could do is to just add a growable flag to the > > BlockDriver to make it explicit. Currently only raw files (in the posix > > and win32 flavours) would set it. > > Good point actually, your current logic is wrong: raw-win32 doesn't have > the file protocol. I'm not sure though if introducing a new flag just > for qemu-io is right. After all qemu-io should test what the drivers > provide to qemu and not extend them to work with qemu-io. Currently the > drivers don't have the concept of non-growable images. True. I'll stick to the current one with the added error return. win32 raw should probably grow a protocol name, but I'll leave that to people who can actually test win32. > > That would also fix the bug what we > > currenly allow to grow host devices even if we can't. > > How that? The host device drivers don't have a protocol name, do they? They don't, I was confused. Updated patch below: Index: qemu/qemu-io.c =================================================================== --- qemu.orig/qemu-io.c 2009-07-06 13:09:45.223364426 +0200 +++ qemu/qemu-io.c 2009-07-06 13:11:41.347269140 +0200 @@ -1172,7 +1172,7 @@ static const cmdinfo_t close_cmd = { .oneline = "close the current open file", }; -static int openfile(char *name, int flags) +static int openfile(char *name, int flags, int growable) { if (bs) { fprintf(stderr, "file open already, try 'help close'\n"); @@ -1189,6 +1189,17 @@ static int openfile(char *name, int flag return 1; } + + if (growable) { + if (!bs->drv || !bs->drv->protocol_name) { + fprintf(stderr, + "%s: only protocols can be opened growable\n", + progname); + return 1; + } + bs->growable = 1; + } + return 0; } @@ -1207,6 +1218,7 @@ open_help(void) " -r, -- open file read-only\n" " -s, -- use snapshot file\n" " -n, -- disable host cache\n" +" -g, -- allow file to grow (only applies to protocols)" "\n"); } @@ -1217,9 +1229,10 @@ open_f(int argc, char **argv) { int flags = 0; int readonly = 0; + int growable = 0; int c; - while ((c = getopt(argc, argv, "snCr")) != EOF) { + while ((c = getopt(argc, argv, "snCrg")) != EOF) { switch (c) { case 's': flags |= BDRV_O_SNAPSHOT; @@ -1233,6 +1246,9 @@ open_f(int argc, char **argv) case 'r': readonly = 1; break; + case 'g': + growable = 1; + break; default: return command_usage(&open_cmd); } @@ -1246,7 +1262,7 @@ open_f(int argc, char **argv) if (optind != argc - 1) return command_usage(&open_cmd); - return openfile(argv[optind], flags); + return openfile(argv[optind], flags, growable); } static const cmdinfo_t open_cmd = { @@ -1306,7 +1322,8 @@ static void usage(const char *name) int main(int argc, char **argv) { int readonly = 0; - const char *sopt = "hVc:Crsnm"; + int growable = 0; + const char *sopt = "hVc:Crsnmg"; struct option lopt[] = { { "help", 0, 0, 'h' }, { "version", 0, 0, 'V' }, @@ -1317,6 +1334,7 @@ int main(int argc, char **argv) { "snapshot", 0, 0, 's' }, { "nocache", 0, 0, 'n' }, { "misalign", 0, 0, 'm' }, + { "growable", 0, 0, 'g' }, { NULL, 0, 0, 0 } }; int c; @@ -1345,6 +1363,9 @@ int main(int argc, char **argv) case 'm': misalign = 1; break; + case 'g': + growable = 1; + break; case 'V': printf("%s version %s\n", progname, VERSION); exit(0); @@ -1392,7 +1413,7 @@ int main(int argc, char **argv) flags |= BDRV_O_RDWR; if ((argc - optind) == 1) - openfile(argv[optind], flags); + openfile(argv[optind], flags, growable); command_loop(); /*