From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [RFC v2] misc/xenmicrocode: Upload /lib/firmware/ to the hypervisor Date: Fri, 30 Jan 2015 16:14:02 -0500 Message-ID: <54CBF41A.3070505@oracle.com> References: <1422580462-26324-1-git-send-email-mcgrof@do-not-panic.com> <54CB98E8.2060701@oracle.com> <20150130200549.GM19988@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YHIu1-0005x5-A4 for xen-devel@lists.xenproject.org; Fri, 30 Jan 2015 21:14:57 +0000 In-Reply-To: <20150130200549.GM19988@wotan.suse.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Luis R. Rodriguez" Cc: Juergen Gross , Michal Marek , Jason Douglas , stefano.stabellini@eu.citrix.com, Takashi Iwai , Andrew Cooper , Henrique de Moraes Holschuh , david.vrabel@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org, "Luis R. Rodriguez" , Borislav Petkov , Olaf Hering List-Id: xen-devel@lists.xenproject.org On 01/30/2015 03:05 PM, Luis R. Rodriguez wrote: > On Fri, Jan 30, 2015 at 09:44:56AM -0500, Boris Ostrovsky wrote: >> On 01/29/2015 08:14 PM, Luis R. Rodriguez wrote: >>> From: "Luis R. Rodriguez" >>> +/* >>> + * Do not pause already paused domains, and allow us to >>> + * unpause only quiesced domains. >>> + */ >>> +static int quiesce_all_domains(xc_interface *xch, >>> + struct xc_quiesce_request *quiesce_request) >>> +{ >>> + xc_domaininfo_t info[1024]; >>> + int i, nb_domain; >>> + >>> + nb_domain = xc_domain_getinfolist(xch, 0, 1024, info); >>> + if ( nb_domain < 0 ) >>> + { >>> + return -1; >>> + } >>> + >>> + for ( i = 0; i < nb_domain; i++ ) >>> + { >>> + if ( info[i].domain == 0 ) >>> + continue; >>> + if ( info[i].flags & XEN_DOMINF_paused ) >>> + continue; >>> + >>> + xc_domain_pause(xch, info[i].domain); >> >> You are not handling errors returned by xc_domain_pause(). > Thanks for the review, shall we just bail if that cannot happen? I think so (after unpausing the already-paused domains, obviously). But then I thought that Andrew is advocating having the hypervisor doing the pause. > >> So then you will >> try to unpause a domain that may not have been paused and (I think more >> importantly) may proceed with microcode update while not all domains are >> paused. > Yeah this would be bad. Perhaps bail and tell the user the domain that > we could not pause / quiesce (depending on what we decide to do). > >>> + >>> + quiesce_request->domids[quiesce_request->num_quiesced] = info[i].domain; >>> + quiesce_request->num_quiesced++; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static void unquiesce_all_domains(xc_interface *xch, >>> + struct xc_quiesce_request *quiesce_request) >>> +{ >>> + int i; >>> + >>> + for ( i = 0; i < quiesce_request->num_quiesced; i++ ) >>> + { >>> + xc_domain_unpause(xch, quiesce_request->domids[i]); >> >> Same here --- you may fail unpausing a domain and noone will know about it. > True, that seems like a rather informational thing we can spit out, do we want > to return an error for that though too? I'd print the error and continue unpausing. -boris