[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 2/2] libxl: add removing XS backend path for PV devices on domain destroy



Roger Pau Monne writes ("Re: [PATCH v1 2/2] libxl: add removing XS backend path 
for PV devices on domain destroy"):
> When this code was added (devd) those where the only backends handled
> by libxl. VDISPL, VSND, VINPUT didn't exist at that point, so I think
> the issue is that when support for those was added, it wasn't properly
> wired into devd.
> 
> > I think:
> > 
> > >      switch(ddev->dev->backend_kind) {
> > > +    case LIBXL__DEVICE_KIND_VDISPL:
> > > +    case LIBXL__DEVICE_KIND_VSND:
> > > +    case LIBXL__DEVICE_KIND_VINPUT:
> > >      case LIBXL__DEVICE_KIND_VBD:
> > >      case LIBXL__DEVICE_KIND_VIF:
> > 
> > If we do want this to handle *all* kinds of device, maybe it should
> > have a fallback that aborts, or something ?  (I don't think it is
> > easy to make it a compile error to forget to add an entry here but if
> > we could do that it would probably be best.)
> 
> Maybe we could have some generic handling for everything != qdisk?

So make that the "default:" ?  That would be fine by me.

> IIRC qdisk needs special handling so that devd can launch and destroy
> a QEMU instance when required, but other backends that run in the
> kernel don't need such handling and could maybe share the same generic
> code path.

Right.

> > All of that assuming that the basic idea is right, which I would like
> > Roger's opinion about.
> 
> Looking at the patch itself, you also seem to be doing some changes
> related to num_vbds and num_vifs, but those are not mentioned in the
> commit message, is that a stray change?

No, I don't think so.  Those variables just tell us when the thing is
torn down but Oleksandr's code is able to use the devices slist itself
for that.  Please do check to see if you agree.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.