From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMM97-00044P-1X for qemu-devel@nongnu.org; Thu, 21 Jan 2016 15:47:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMM96-0002xf-4C for qemu-devel@nongnu.org; Thu, 21 Jan 2016 15:47:56 -0500 References: <1453375338-13508-1-git-send-email-rudyflyzhang@gmail.com> <1453375338-13508-2-git-send-email-rudyflyzhang@gmail.com> <56A109D1.4000701@redhat.com> From: John Snow Message-ID: <56A143F4.7080601@redhat.com> Date: Thu, 21 Jan 2016 15:47:48 -0500 MIME-Version: 1.0 In-Reply-To: <56A109D1.4000701@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] hmp: add hmp command for incremental backup List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , Rudy Zhang , qemu-devel@nongnu.org Cc: famz@redhat.com, qemu-block@nongnu.org On 01/21/2016 11:39 AM, Eric Blake wrote: > On 01/21/2016 04:22 AM, Rudy Zhang wrote: >> Add hmp command for incremental backup in drive-backup. >> It need a bitmap to backup data from drive-image to incremental image, >> so before it need add bitmap for this device to track io. >> Usage: >> drive_backup [-n] [-f] device target [bitmap] [format] >> >> Signed-off-by: Rudy Zhang >> --- >> hmp-commands.hx | 5 +++-- >> hmp.c | 16 ++++++++++++++-- >> 2 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index bb52e4d..7378aaa 100644 >> --- a/hmp-commands.hx >> +++ b/hmp-commands.hx >> @@ -1180,12 +1180,13 @@ ETEXI >> =20 >> { >> .name =3D "drive_backup", >> - .args_type =3D "reuse:-n,full:-f,device:B,target:s,format:s?= ", >> - .params =3D "[-n] [-f] device target [format]", >> + .args_type =3D "reuse:-n,full:-f,device:B,target:s,bitmap:s?= ,format:s?", >> + .params =3D "[-n] [-f] device target [bitmap] [format]", >=20 > This is HMP, so it may not matter, but this is not backwards compatible= . > Scripts targetting the old style of passing a format will now have tha= t > format string interpreted as a bitmap name with no format. Better woul= d > be to stick [bitmap] at the end, not the middle. >=20 I'd like to also point out that the method of intuiting the backup mode based on the parameters present won't expand very well as we add more modes, especially if there's ever an addition for a differential backup mode. Maybe it's fine because it's HMP and backwards compatibility is not a real concern ... >=20 >> @@ -1098,6 +1100,17 @@ void hmp_drive_backup(Monitor *mon, const QDict= *qdict) >> return; >> } >> =20 >> + if (full && bitmap) { >> + error_setg(&err, "Parameter 'bitmap' if conflict with '-f'"); >=20 > s/if conflict/conflicts/ >=20 >> + hmp_handle_error(mon, &err); >> + return; >> + } else if (full) >> + sync =3D MIRROR_SYNC_MODE_FULL; >=20 > Needs {}. Run your patch through scripts/checkpatch.pl, to flag this > and other style violations. >=20 --=20 =E2=80=94js