From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33578) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SySEU-0001m0-5F for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:40:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SySES-000289-Sg for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:40:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59737) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SySES-00027P-Jf for qemu-devel@nongnu.org; Mon, 06 Aug 2012 14:40:48 -0400 Message-ID: <501FF0B4.6040309@redhat.com> Date: Mon, 06 Aug 2012 19:28:36 +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> In-Reply-To: <501FED67.7070809@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:14 PM, Eric Blake wrote: > On 08/06/2012 10:04 AM, Orit Wasserman wrote: >> On 08/06/2012 05:26 PM, Eric Blake wrote: >>> On 08/05/2012 03:13 AM, Orit Wasserman wrote: >>>> The management can enable/disable a capability for the next migration by using >>>> migrate-set-apabilities QMP command. >>> >>> s/set-apabilities/set-capabilities/ >>> > >>> In HMP, are migrate_supported_capabilities and migrate_capabilities >>> redundant? That is, I think I can use either command to answer both >>> questions "what capabilities exist" and "what is the current state of >>> all capabilities that exist", since _both_ commands output a list of >>> capability names as well as an on/off designator. If my analysis is >>> right, then we don't need migrate_supported_capabilities. >> No 'info migrate_supported_capabilities' shows the capabilities this version of QEMU can supports. >> and 'info migrate_capabilities' show what are the state of capabilities for the migration, i.e what is enabled. > > Let's compare: > > patch 1/11: > > +void hmp_info_migrate_supported_capabilities(Monitor *mon) > +{ > + MigrationCapabilityStatusList *caps_list, *cap; > + > + caps_list = qmp_query_migrate_supported_capabilities(NULL); > + if (!caps_list) { > + monitor_printf(mon, "No supported migration capabilities found\n"); > + return; > + } > + > + for (cap = caps_list; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > > > patch 2/11: > > +void hmp_info_migrate_capabilities(Monitor *mon) > +{ > + MigrationCapabilityStatusList *caps, *cap; > + > + caps = qmp_query_migrate_capabilities(NULL); > + > + if (caps) { > + monitor_printf(mon, "capabilities: "); > + for (cap = caps; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + > MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > > 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. > > They really ARE redundant - both commands are telling me: > > capabilities: > xbzrle: on > foobar: off > > which I can read to answer both my question of 'what is supported' > (xbzrle and foobar) and 'what is enabled' (xbzrle). I see no need to > have to commands to tell me the same information, so I'd prefer the > shorter name. > 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) { 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; } 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 Orit