Re: [Xen-devel] [PATCH] xenconsoled: use grant references instead of map_foreign_range

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

> >> +  if (!dom->interface) {
> >> +          /* Fall back to xc_map_foreign_range */
> > 
> > If you get here and then xc_map_foreign_range fails nothing will reset
> > dom->ring_ref to -1, so in the case where it has changed and you are
> > remapping it will retain its previous value. That's going to cause
> > confusion if the ring ref changes again I think? Easily fixed by
> > resetting ring_ref in domain_unmap_interface.
> 
> Yep, will add that next version.

Thanks.

> > Hrm, that's probably a preexisting issue now I think about it.
> >
> > I notice that nothing checks the error returns from this function. Oh
> > well.
> > 
> >>    d->xce_handle = NULL;
> >> @@ -736,6 +757,13 @@ void enum_domains(void)
> >>    xc_dominfo_t dominfo;
> >>    struct domain *dom;
> >>  
> >> +  if (enum_pass == 0) {
> >> +          xcg_handle = xc_gnttab_open(NULL, 0);
> >> +          if (xcg_handle == NULL) {
> >> +                  dolog(LOG_DEBUG, "Failed to open xcg handle: %d (%s)",
> >> +                            errno, strerror(errno));
> >> +          }
> >> +  }
> > 
> > I think it would be preferable to open this along with the other handles
> > in handle_io.
> >
> That's too late: the grant table file descriptor must be open when 
> enum_domains
> is run, and main() calls enum_domains() just before it calls handle_io().

OK.

> I could move that call to enum_domains() into handle_io() if that's preferred;
> it seems like that would be cleaner, especially since that's where the cleanup
> is located.

Either that or moving the xc_gnttab_open and cleanup into main() would
be good I think.

> 



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