Re: [Xen-devel] [RFC v2] misc/xenmicrocode: Upload /lib/firmware/<some blob> to the hypervisor

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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" <mcgrof@xxxxxxxx>
> > > +/*
> > > + * 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel