From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SySHW-0000RV-C1 for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:43:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SySHU-0003OY-UE for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:43:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50691) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SySHU-0003OM-MJ for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:43:56 -0400 Message-ID: <501FF591.1070309@redhat.com> Date: Mon, 06 Aug 2012 19:49:21 +0300 From: Orit Wasserman MIME-Version: 1.0 References: <1344158004-10370-1-git-send-email-owasserm@redhat.com> <1344158004-10370-3-git-send-email-owasserm@redhat.com> <501FD40E.8030506@redhat.com> <501FEB19.3030007@redhat.com> <501FED67.7070809@redhat.com> <501FF0B4.6040309@redhat.com> <501FF348.7020100@redhat.com> In-Reply-To: <501FF348.7020100@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/11] Add migrate-set-capabilities and query-migrate-capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, quintela@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, blauwirbel@gmail.com, avi@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, chegu_vinod@hp.com On 08/06/2012 07:39 PM, Eric Blake wrote: > On 08/06/2012 10:28 AM, Orit Wasserman wrote: > >>> That is, BOTH commands end up iterating over a list of caps, and output >>> identical information in the case where caps exist of 'name: state' for >>> each capability. >>> > >> The information is different: >> the first: >> MigrationCapabilityStatusList * >> qmp_query_migrate_supported_capabilities(Error **errp) >> { >> MigrationCapabilityStatusList *caps_list = g_malloc0(sizeof(*caps_list)); >> >> caps_list->value = g_malloc(sizeof(*caps_list->value)); >> caps_list->value->capability = MIGRATION_CAPABILITY_XBZRLE; >> caps_list->value->state = true; >> caps_list->next = NULL; >> >> return caps_list; >> } >> >> the second: >> MigrationCapabilityStatusList * >> qmp_query_migrate_supported_capabilities(Error **errp) > > I think you meant this for the second: > > +MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp) > +{ > + MigrationCapabilityStatusList *head = NULL; > + MigrationCapabilityStatusList *caps; > + MigrationState *s = migrate_get_current(); > + int i; > + > + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + if (head == NULL) { > + head = g_malloc0(sizeof(*caps)); > + caps = head; > + } else { > + caps->next = g_malloc0(sizeof(*caps)); > + caps = caps->next; > + } > + caps->value = > + g_malloc(sizeof(*caps->value)); > + caps->value->capability = i; > + caps->value->state = s->enabled_capabilities[i]; > + } > >> you can look at it as 64bit support one is to know if the processor supports 64 bit >> the other to know if the OS uses 64 bit > > Okay, so the QMP code is currently different (one outputs a list where > every entry in the list is hard-coded to true, the other outputs a list > where each entry reflects the current state). But that still doesn't > address my concern that you don't need two QMP commands. > > Merely listing 'xbzrle' in the list is enough to know that 'this > particular qemu knows how to do xbzrle', and the user is free to ignore > the additional information of whether it is actually in use at the time > if all the application cared about was determining the set of known > capabilities. > > Given your argument with 64-bit processing, the same reasoning applies - > ask for a list of capabilities, and the result will either omit '64bit' > (the capability is not possible), include '64bit:false' (the capability > is known by the emulator, but not in use by the guest), or include > '64bit:true' (the capability is both known and in-use). A user that > only cares about querying supported capabilities will check whether > '64bit' is in the list, and throw away the information about whether it > is on or off (and since the QMP command currently returns a hard-coded > true, that information is already being discarded via your > query-migrate-supported-capabilities command). That is, your > implementation for query-migrate-capabilities is sufficient to satisfy > both class of users, without needing query-migrate-supported-capabilities. > As you represent the user (libvirt) you know the best. I will remove supported-capabilities commands. Orit