| Message ID | 1500387930-16317-7-git-send-email-al1img@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: [...] > /* Waits for the passed device to reach state XenbusStateInitWait. > * This is not really useful by itself, but is important when executing > * hotplug scripts, since we need to be sure the device is in the correct > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; > extern const struct libxl_device_type libxl__usbdev_devtype; > extern const struct libxl_device_type libxl__pcidev_devtype; > extern const struct libxl_device_type libxl__vdispl_devtype; > +extern const struct libxl_device_type libxl__p9_devtype; > > extern const struct libxl_device_type *device_type_tbl[]; > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 25563cf..96dbaed 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), > - ("p9", Array(libxl_device_p9, "num_p9s")), > + ("p9s", Array(libxl_device_p9, "num_p9s")), Oh, no, please don't do this. We can't change the name of the fields. There is already on irregular device type -- the PCI device. I suppose you probably need another hook somewhere. And please convert PCI devices if you can.
On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: > On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: > [...] > > /* Waits for the passed device to reach state XenbusStateInitWait. > > * This is not really useful by itself, but is important when executing > > * hotplug scripts, since we need to be sure the device is in the correct > > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; > > extern const struct libxl_device_type libxl__usbdev_devtype; > > extern const struct libxl_device_type libxl__pcidev_devtype; > > extern const struct libxl_device_type libxl__vdispl_devtype; > > +extern const struct libxl_device_type libxl__p9_devtype; > > > > extern const struct libxl_device_type *device_type_tbl[]; > > > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 25563cf..96dbaed 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ > > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), > > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), > > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), > > - ("p9", Array(libxl_device_p9, "num_p9s")), > > + ("p9s", Array(libxl_device_p9, "num_p9s")), > > Oh, no, please don't do this. We can't change the name of the fields. > > There is already on irregular device type -- the PCI device. I suppose > you probably need another hook somewhere. And please convert PCI devices > if you can. OK, going through the code I think we need to come to a conclusion if we want an extra callback to handle the irregular device names first because that's likely to affect the code of the framework in previous patch.
On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: >> [...] >> > /* Waits for the passed device to reach state XenbusStateInitWait. >> > * This is not really useful by itself, but is important when executing >> > * hotplug scripts, since we need to be sure the device is in the correct >> > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; >> > extern const struct libxl_device_type libxl__usbdev_devtype; >> > extern const struct libxl_device_type libxl__pcidev_devtype; >> > extern const struct libxl_device_type libxl__vdispl_devtype; >> > +extern const struct libxl_device_type libxl__p9_devtype; >> > >> > extern const struct libxl_device_type *device_type_tbl[]; >> > >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> > index 25563cf..96dbaed 100644 >> > --- a/tools/libxl/libxl_types.idl >> > +++ b/tools/libxl/libxl_types.idl >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), >> > - ("p9", Array(libxl_device_p9, "num_p9s")), >> > + ("p9s", Array(libxl_device_p9, "num_p9s")), >> >> Oh, no, please don't do this. We can't change the name of the fields. >> >> There is already on irregular device type -- the PCI device. I suppose >> you probably need another hook somewhere. And please convert PCI devices >> if you can. > > OK, going through the code I think we need to come to a conclusion if we > want an extra callback to handle the irregular device names first > because that's likely to affect the code of the framework in previous > patch. Actually creating new callback to handle irregular device name looks not so good. There is the pattern which all namings should follow. May be it has to be documented somewhere. p9 was added recently we can ask the author to review this rename. From other side this rename touches only internals changes: no changes in config file or CLI interface.
On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote: > On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote: > > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: > >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: > >> [...] > >> > /* Waits for the passed device to reach state XenbusStateInitWait. > >> > * This is not really useful by itself, but is important when executing > >> > * hotplug scripts, since we need to be sure the device is in the correct > >> > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; > >> > extern const struct libxl_device_type libxl__usbdev_devtype; > >> > extern const struct libxl_device_type libxl__pcidev_devtype; > >> > extern const struct libxl_device_type libxl__vdispl_devtype; > >> > +extern const struct libxl_device_type libxl__p9_devtype; > >> > > >> > extern const struct libxl_device_type *device_type_tbl[]; > >> > > >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > >> > index 25563cf..96dbaed 100644 > >> > --- a/tools/libxl/libxl_types.idl > >> > +++ b/tools/libxl/libxl_types.idl > >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ > >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), > >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), > >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), > >> > - ("p9", Array(libxl_device_p9, "num_p9s")), > >> > + ("p9s", Array(libxl_device_p9, "num_p9s")), > >> > >> Oh, no, please don't do this. We can't change the name of the fields. > >> > >> There is already on irregular device type -- the PCI device. I suppose > >> you probably need another hook somewhere. And please convert PCI devices > >> if you can. > > > > OK, going through the code I think we need to come to a conclusion if we > > want an extra callback to handle the irregular device names first > > because that's likely to affect the code of the framework in previous > > patch. > > Actually creating new callback to handle irregular device name looks > not so good. > There is the pattern which all namings should follow. May be it has to > be documented The normal pattern is DEVTYPEs. > somewhere. p9 was added recently we can ask the author to review this rename. Once it is released we can't change it, of course unless we deem it unstable. I'm two minded here. P9 was released in 4.9, which was only a few months old. But we definitely can't change the PCI type. It has been around since forever. And there is provision in code to deal with that. > From other side this rename touches only internals changes: no changes > in config file > or CLI interface. > As said, the framework need to be ready to deal with PCI anyway. What sort of issues do you foresee here?
On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote: >> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote: >> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: >> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: >> >> [...] >> >> > /* Waits for the passed device to reach state XenbusStateInitWait. >> >> > * This is not really useful by itself, but is important when executing >> >> > * hotplug scripts, since we need to be sure the device is in the correct >> >> > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; >> >> > extern const struct libxl_device_type libxl__usbdev_devtype; >> >> > extern const struct libxl_device_type libxl__pcidev_devtype; >> >> > extern const struct libxl_device_type libxl__vdispl_devtype; >> >> > +extern const struct libxl_device_type libxl__p9_devtype; >> >> > >> >> > extern const struct libxl_device_type *device_type_tbl[]; >> >> > >> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> >> > index 25563cf..96dbaed 100644 >> >> > --- a/tools/libxl/libxl_types.idl >> >> > +++ b/tools/libxl/libxl_types.idl >> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ >> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), >> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), >> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), >> >> > - ("p9", Array(libxl_device_p9, "num_p9s")), >> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")), >> >> >> >> Oh, no, please don't do this. We can't change the name of the fields. >> >> >> >> There is already on irregular device type -- the PCI device. I suppose >> >> you probably need another hook somewhere. And please convert PCI devices >> >> if you can. >> > >> > OK, going through the code I think we need to come to a conclusion if we >> > want an extra callback to handle the irregular device names first >> > because that's likely to affect the code of the framework in previous >> > patch. >> >> Actually creating new callback to handle irregular device name looks >> not so good. >> There is the pattern which all namings should follow. May be it has to >> be documented > > The normal pattern is DEVTYPEs. > >> somewhere. p9 was added recently we can ask the author to review this rename. > > Once it is released we can't change it, of course unless we deem it > unstable. I'm two minded here. P9 was released in 4.9, which was only a > few months old. > IMHO this the bug I mean wrong naming and it should be fixed. > But we definitely can't change the PCI type. It has been around since > forever. And there is provision in code to deal with that. Agree, I didn't touch PCI. >> From other side this rename touches only internals changes: no changes >> in config file >> or CLI interface. >> > > As said, the framework need to be ready to deal with PCI anyway. > > What sort of issues do you foresee here? Do you mean in case to rework PCI to use the device framework?
On Tue, Aug 01, 2017 at 02:58:19PM +0300, Oleksandr Grytsov wrote: > On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote: > > On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote: > >> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote: > >> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: > >> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: > >> >> [...] > >> >> > /* Waits for the passed device to reach state XenbusStateInitWait. > >> >> > * This is not really useful by itself, but is important when executing > >> >> > * hotplug scripts, since we need to be sure the device is in the correct > >> >> > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; > >> >> > extern const struct libxl_device_type libxl__usbdev_devtype; > >> >> > extern const struct libxl_device_type libxl__pcidev_devtype; > >> >> > extern const struct libxl_device_type libxl__vdispl_devtype; > >> >> > +extern const struct libxl_device_type libxl__p9_devtype; > >> >> > > >> >> > extern const struct libxl_device_type *device_type_tbl[]; > >> >> > > >> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > >> >> > index 25563cf..96dbaed 100644 > >> >> > --- a/tools/libxl/libxl_types.idl > >> >> > +++ b/tools/libxl/libxl_types.idl > >> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ > >> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), > >> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), > >> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), > >> >> > - ("p9", Array(libxl_device_p9, "num_p9s")), > >> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")), > >> >> > >> >> Oh, no, please don't do this. We can't change the name of the fields. > >> >> > >> >> There is already on irregular device type -- the PCI device. I suppose > >> >> you probably need another hook somewhere. And please convert PCI devices > >> >> if you can. > >> > > >> > OK, going through the code I think we need to come to a conclusion if we > >> > want an extra callback to handle the irregular device names first > >> > because that's likely to affect the code of the framework in previous > >> > patch. > >> > >> Actually creating new callback to handle irregular device name looks > >> not so good. > >> There is the pattern which all namings should follow. May be it has to > >> be documented > > > > The normal pattern is DEVTYPEs. > > > >> somewhere. p9 was added recently we can ask the author to review this rename. > > > > Once it is released we can't change it, of course unless we deem it > > unstable. I'm two minded here. P9 was released in 4.9, which was only a > > few months old. > > > > IMHO this the bug I mean wrong naming and it should be fixed. > > > But we definitely can't change the PCI type. It has been around since > > forever. And there is provision in code to deal with that. > > Agree, I didn't touch PCI. > > >> From other side this rename touches only internals changes: no changes > >> in config file > >> or CLI interface. > >> > > > > As said, the framework need to be ready to deal with PCI anyway. > > > > What sort of issues do you foresee here? > > Do you mean in case to rework PCI to use the device framework? > No, I mean adding the new hook. You said "handle irregular device name looks not so good"
On Tue, Aug 1, 2017 at 4:00 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Tue, Aug 01, 2017 at 02:58:19PM +0300, Oleksandr Grytsov wrote: >> On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote: >> > On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote: >> >> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote: >> >> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote: >> >> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: >> >> >> [...] >> >> >> > /* Waits for the passed device to reach state XenbusStateInitWait. >> >> >> > * This is not really useful by itself, but is important when executing >> >> >> > * hotplug scripts, since we need to be sure the device is in the correct >> >> >> > @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; >> >> >> > extern const struct libxl_device_type libxl__usbdev_devtype; >> >> >> > extern const struct libxl_device_type libxl__pcidev_devtype; >> >> >> > extern const struct libxl_device_type libxl__vdispl_devtype; >> >> >> > +extern const struct libxl_device_type libxl__p9_devtype; >> >> >> > >> >> >> > extern const struct libxl_device_type *device_type_tbl[]; >> >> >> > >> >> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> >> >> > index 25563cf..96dbaed 100644 >> >> >> > --- a/tools/libxl/libxl_types.idl >> >> >> > +++ b/tools/libxl/libxl_types.idl >> >> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ >> >> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")), >> >> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")), >> >> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), >> >> >> > - ("p9", Array(libxl_device_p9, "num_p9s")), >> >> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")), >> >> >> >> >> >> Oh, no, please don't do this. We can't change the name of the fields. >> >> >> >> >> >> There is already on irregular device type -- the PCI device. I suppose >> >> >> you probably need another hook somewhere. And please convert PCI devices >> >> >> if you can. >> >> > >> >> > OK, going through the code I think we need to come to a conclusion if we >> >> > want an extra callback to handle the irregular device names first >> >> > because that's likely to affect the code of the framework in previous >> >> > patch. >> >> >> >> Actually creating new callback to handle irregular device name looks >> >> not so good. >> >> There is the pattern which all namings should follow. May be it has to >> >> be documented >> > >> > The normal pattern is DEVTYPEs. >> > >> >> somewhere. p9 was added recently we can ask the author to review this rename. >> > >> > Once it is released we can't change it, of course unless we deem it >> > unstable. I'm two minded here. P9 was released in 4.9, which was only a >> > few months old. >> > >> >> IMHO this the bug I mean wrong naming and it should be fixed. >> >> > But we definitely can't change the PCI type. It has been around since >> > forever. And there is provision in code to deal with that. >> >> Agree, I didn't touch PCI. >> >> >> From other side this rename touches only internals changes: no changes >> >> in config file >> >> or CLI interface. >> >> >> > >> > As said, the framework need to be ready to deal with PCI anyway. >> > >> > What sort of issues do you foresee here? >> >> Do you mean in case to rework PCI to use the device framework? >> > > No, I mean adding the new hook. You said "handle irregular device name > looks not so good" As for me when only internal name of structure or fields are changed then it should not break anyone configs, setup etc. Creating hooks in this case makes code hard to read and maintain.
On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote: [...] > >> >> From other side this rename touches only internals changes: no changes > >> >> in config file > >> >> or CLI interface. > >> >> > >> > > >> > As said, the framework need to be ready to deal with PCI anyway. > >> > > >> > What sort of issues do you foresee here? > >> > >> Do you mean in case to rework PCI to use the device framework? > >> > > > > No, I mean adding the new hook. You said "handle irregular device name > > looks not so good" > > As for me when only internal name of structure or fields are changed > then it should not break anyone configs, setup etc. > Creating hooks in this case makes code hard to read and maintain. > I think you missed my points: 1. libxl types generated from libxl_types.idl aren't just used by xl. Some applications will use libxl types directly. Not breaking xl config doesn't mean not breaking other toolstacks like libvirt. In this particular case, I think we might be able to change p9 to p9s because it is only released a few months back because the only other known toolstack that uses libxl can't possibly use that field at the moment. But Ian might disagree. 2. There is another type, pci dev, that has been there since forever. We need a hook to deal with it, we can't rename it. 1 and 2 are orthogonal. 2 is a hard requirement.
On Fri, Aug 4, 2017 at 2:53 PM, Wei Liu <wei.liu2@citrix.com> wrote: > On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote: > [...] >> >> >> From other side this rename touches only internals changes: no changes >> >> >> in config file >> >> >> or CLI interface. >> >> >> >> >> > >> >> > As said, the framework need to be ready to deal with PCI anyway. >> >> > >> >> > What sort of issues do you foresee here? >> >> >> >> Do you mean in case to rework PCI to use the device framework? >> >> >> > >> > No, I mean adding the new hook. You said "handle irregular device name >> > looks not so good" >> >> As for me when only internal name of structure or fields are changed >> then it should not break anyone configs, setup etc. >> Creating hooks in this case makes code hard to read and maintain. >> > > I think you missed my points: > > 1. libxl types generated from libxl_types.idl aren't just used by xl. > Some applications will use libxl types directly. Not breaking xl config > doesn't mean not breaking other toolstacks like libvirt. In this > particular case, I think we might be able to change p9 to p9s because it > is only released a few months back because the only other known > toolstack that uses libxl can't possibly use that field at the moment. > But Ian might disagree. I got it. I think that we have to change p9 to p9s ASAP to avoid future hooks. > 2. There is another type, pci dev, that has been there since forever. We > need a hook to deal with it, we can't rename it. > For PCI all hooks are already there (DEFINE_DEVICE_TYPE_STRUCT_X to handle pcidev and pci). Also I didn't change PCI fields, names etc. In libxl_domain_config it is already pcidevs. So, we are safe with PCI. Sorry I don't understand your concern about PCI. Or I miss something? > 1 and 2 are orthogonal. 2 is a hard requirement.
On Tue, Aug 08, 2017 at 03:39:23PM +0300, Oleksandr Grytsov wrote: > On Fri, Aug 4, 2017 at 2:53 PM, Wei Liu <wei.liu2@citrix.com> wrote: > > On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote: > > [...] > >> >> >> From other side this rename touches only internals changes: no changes > >> >> >> in config file > >> >> >> or CLI interface. > >> >> >> > >> >> > > >> >> > As said, the framework need to be ready to deal with PCI anyway. > >> >> > > >> >> > What sort of issues do you foresee here? > >> >> > >> >> Do you mean in case to rework PCI to use the device framework? > >> >> > >> > > >> > No, I mean adding the new hook. You said "handle irregular device name > >> > looks not so good" > >> > >> As for me when only internal name of structure or fields are changed > >> then it should not break anyone configs, setup etc. > >> Creating hooks in this case makes code hard to read and maintain. > >> > > > > I think you missed my points: > > > > 1. libxl types generated from libxl_types.idl aren't just used by xl. > > Some applications will use libxl types directly. Not breaking xl config > > doesn't mean not breaking other toolstacks like libvirt. In this > > particular case, I think we might be able to change p9 to p9s because it > > is only released a few months back because the only other known > > toolstack that uses libxl can't possibly use that field at the moment. > > But Ian might disagree. > > I got it. I think that we have to change p9 to p9s ASAP to avoid future hooks. > > > 2. There is another type, pci dev, that has been there since forever. We > > need a hook to deal with it, we can't rename it. > > > > For PCI all hooks are already there (DEFINE_DEVICE_TYPE_STRUCT_X > to handle pcidev and pci). Also I didn't change PCI fields, names etc. > In libxl_domain_config it is already pcidevs. So, we are safe with PCI. > Sorry I don't understand your concern about PCI. Or I miss something? Yes I think we're covered there. That macro only covers the case where new characters are appended. I was thinking maybe we should have something that deal with new names weather they are longer or shorter than expected. But I see now it is probably better to rename the p9 device. I will send out an email asking for opinions. > > > 1 and 2 are orthogonal. 2 is a hard requirement. > > > -- > Best Regards, > Oleksandr Grytsov.
On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote: > From: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> This needs rebasing.
diff --git a/tools/libxl/libxl_9pfs.c b/tools/libxl/libxl_9pfs.c index 07e3e5f..394d8b4 100644 --- a/tools/libxl/libxl_9pfs.c +++ b/tools/libxl/libxl_9pfs.c @@ -17,12 +17,10 @@ #include "libxl_internal.h" -int libxl__device_p9_setdefault(libxl__gc *gc, libxl_device_p9 *p9) +static int libxl__device_p9_setdefault(libxl__gc *gc, uint32_t domid, + libxl_device_p9 *p9, bool hotplug) { - int rc; - - rc = libxl__resolve_domid(gc, p9->backend_domname, &p9->backend_domid); - return rc; + return libxl__resolve_domid(gc, p9->backend_domname, &p9->backend_domid); } static int libxl__device_from_p9(libxl__gc *gc, uint32_t domid, @@ -39,49 +37,30 @@ static int libxl__device_from_p9(libxl__gc *gc, uint32_t domid, return 0; } - -int libxl__device_p9_add(libxl__gc *gc, uint32_t domid, - libxl_device_p9 *p9) +static int libxl__set_xenstore_p9(libxl__gc *gc, uint32_t domid, + libxl_device_p9 *p9, + flexarray_t *back, flexarray_t *front, + flexarray_t *ro_front) { - flexarray_t *front; - flexarray_t *back; - libxl__device device; - int rc; - - rc = libxl__device_p9_setdefault(gc, p9); - if (rc) goto out; - - front = flexarray_make(gc, 16, 1); - back = flexarray_make(gc, 16, 1); - - if (p9->devid == -1) { - if ((p9->devid = libxl__device_nextid(gc, domid, "9pfs")) < 0) { - rc = ERROR_FAIL; - goto out; - } - } - - rc = libxl__device_from_p9(gc, domid, p9, &device); - if (rc != 0) goto out; - - flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d", domid)); - flexarray_append_pair(back, "online", "1"); - flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising)); - flexarray_append_pair(front, "backend-id", - libxl__sprintf(gc, "%d", p9->backend_domid)); - flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising)); - flexarray_append_pair(front, "tag", p9->tag); flexarray_append_pair(back, "path", p9->path); flexarray_append_pair(back, "security_model", p9->security_model); - libxl__device_generic_add(gc, XBT_NULL, &device, - libxl__xs_kvs_of_flexarray(gc, back), - libxl__xs_kvs_of_flexarray(gc, front), - NULL); - rc = 0; -out: - return rc; + flexarray_append_pair(front, "tag", p9->tag); + + return 0; } -LIBXL_DEFINE_DEVICE_REMOVE(p9) +#define libxl__add_p9s NULL +#define libxl_device_p9_list NULL +#define libxl_device_p9_compare NULL +LIBXL_DEFINE_DEVICE_REMOVE(p9) +static LIBXL_DEFINE_UPDATE_DEVID(p9, "9pfs") + +DEFINE_DEVICE_TYPE_STRUCT(p9, + .skip_attach = 1, + .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *, + flexarray_t *back, flexarray_t *front, + flexarray_t *ro_front)) + libxl__set_xenstore_p9 +); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 912bd21..f483475 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -1325,7 +1325,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, } for (i = 0; i < d_config->num_p9s; i++) - libxl__device_p9_add(gc, domid, &d_config->p9[i]); + libxl__device_add(gc, domid, &libxl__p9_devtype, &d_config->p9s[i]); switch (d_config->c_info.type) { case LIBXL_DOMAIN_TYPE_HVM: diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 68c08aa..c53bbd1 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1252,8 +1252,6 @@ _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb); _hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci); _hidden void libxl__rdm_setdefault(libxl__gc *gc, libxl_domain_build_info *b_info); -_hidden int libxl__device_p9_setdefault(libxl__gc *gc, - libxl_device_p9 *p9); _hidden const char *libxl__device_nic_devname(libxl__gc *gc, uint32_t domid, @@ -2668,10 +2666,6 @@ _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid, _hidden int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb); -/* Internal function to connect a 9pfs device */ -_hidden int libxl__device_p9_add(libxl__gc *gc, uint32_t domid, - libxl_device_p9 *p9); - /* Waits for the passed device to reach state XenbusStateInitWait. * This is not really useful by itself, but is important when executing * hotplug scripts, since we need to be sure the device is in the correct @@ -3565,6 +3559,7 @@ extern const struct libxl_device_type libxl__usbctrl_devtype; extern const struct libxl_device_type libxl__usbdev_devtype; extern const struct libxl_device_type libxl__pcidev_devtype; extern const struct libxl_device_type libxl__vdispl_devtype; +extern const struct libxl_device_type libxl__p9_devtype; extern const struct libxl_device_type *device_type_tbl[]; diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 25563cf..96dbaed 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [ ("vfbs", Array(libxl_device_vfb, "num_vfbs")), ("vkbs", Array(libxl_device_vkb, "num_vkbs")), ("vtpms", Array(libxl_device_vtpm, "num_vtpms")), - ("p9", Array(libxl_device_p9, "num_p9s")), + ("p9s", Array(libxl_device_p9, "num_p9s")), ("vdispls", Array(libxl_device_vdispl, "num_vdispls")), # a channel manifests as a console with a name, # see docs/misc/channels.txt diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 2140905..19eadd9 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1419,9 +1419,9 @@ void parse_config_data(const char *config_source, char *p, *p2, *buf2; d_config->num_p9s = 0; - d_config->p9 = NULL; + d_config->p9s = NULL; while ((buf = xlu_cfg_get_listitem (p9devs, d_config->num_p9s)) != NULL) { - p9 = ARRAY_EXTEND_INIT(d_config->p9, + p9 = ARRAY_EXTEND_INIT(d_config->p9s, d_config->num_p9s, libxl_device_p9_init); libxl_device_p9_init(p9);