From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49391) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UdHjs-00066R-Pk for qemu-devel@nongnu.org; Fri, 17 May 2013 06:18:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UdHjr-0008Dt-CY for qemu-devel@nongnu.org; Fri, 17 May 2013 06:18:16 -0400 Received: from cantor2.suse.de ([195.135.220.15]:56035 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UdHjr-0008Bh-45 for qemu-devel@nongnu.org; Fri, 17 May 2013 06:18:15 -0400 Message-ID: <519603E2.5060107@suse.de> Date: Fri, 17 May 2013 12:18:10 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1366731014-48790-1-git-send-email-jfrei@linux.vnet.ibm.com> <1366731014-48790-2-git-send-email-jfrei@linux.vnet.ibm.com> <5176ABB7.8080102@redhat.com> <20130423155428.GA15345@linux.vnet.ibm.com> In-Reply-To: <20130423155428.GA15345@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jens Freimann Cc: Peter Maydell , Ekaterina Tumanova , qemu-devel , Alexander Graf , Rabin Vincent , Christian Borntraeger , Paolo Bonzini Am 23.04.2013 17:54, schrieb Jens Freimann: > On Tue, Apr 23, 2013 at 09:41:43AM -0600, Eric Blake wrote: >> On 04/23/2013 09:30 AM, Jens Freimann wrote: >>> Split out dump-guest-memory memory mapping code to allow dumping with= out >>> memory mapping >>> >>> The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well= as >>> CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and with= out paging. >>> Some architectures will provide only the non-paging case. This patch = allows an >>> architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPP= ING is not >>> available. To do that, we split out the common code and provide stub = functions >>> for the non-paging case. If -p is specified on a target that doesn't = support it, >>> we will pass an error to the calling code. >>> >>> Signed-off-by: Ekaterina Tumanova >>> Signed-off-by: Jens Freimann >>> --- >> >>> +++ b/include/qapi/qmp/qerror.h >>> @@ -249,4 +249,7 @@ void assert_no_error(Error *err); >>> #define QERR_SOCKET_CREATE_FAILED \ >>> ERROR_CLASS_GENERIC_ERROR, "Failed to create socket" >>> =20 >>> +#define QERR_UNSUPPORTED_COMMAND_OPTION \ >>> + ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not suppo= rted for %s" >> >> Rather than adding a new QERR_* constant here, just use error_setg() i= n >> qmp_dump_guest_memory() in the first place. >=20 > ok, will fix Any progress on this? I'd be happy to take an updated version of this generic patch and leave the s390x part for Alex et al. to review. If you resend, note that Git complained about a trailing white line in the new file; otherwise I'd be fine with a diff+Sob or suggestion for the error_setg() text. Thanks. >> This raises an interesting question about introspection - how will >> management apps (such as libvirt) be able to determine whether the >> paging command is supported for a given architecture? Do we need to >> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tel= l >> us whether a given machine will support or reject attempts to set >> 'paging':true during 'dump-guest-memory'? >=20 > sounds reasonable to me. This discussion seems orthogonal since we are not changing behavior. AFAICS we were only hiding the documentation of the dump-guest-memory command but not shielding its implementation and the stub raised QERR_UNSUPPORTED, so raising an error when the implementation doesn't support paging doesn't seem a regression to me and can be revisited when we actually add a second implementation, whether s390x or arm. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg