| Message ID | 20170904172500.26513-1-n54@gmx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4 September 2017 at 18:25, Kamil Rytarowski <n54@gmx.com> wrote: > This fixes build on SmartOS (Joyent). > > Patch cherry-picked from pkgsrc by jperkin (Joyent). > > Signed-off-by: Kamil Rytarowski <n54@gmx.com> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > include/qemu/osdep.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 6855b94bbf..5d3860f80e 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -306,6 +306,11 @@ void qemu_anon_ram_free(void *ptr, size_t size); > #endif > #endif > > +/* Required by SmartOS (SunOS) */ > +#ifndef NAME_MAX > +#define NAME_MAX 255 > +#endif So in hw/usb/dev-mtp.c we're using NAME_MAX in char buf[sizeof(struct inotify_event) + NAME_MAX + 1]; because the Linux implementation of inotify documents in inotify(7) that this is guaranteed to be sufficient to read at least one event: http://man7.org/linux/man-pages/man7/inotify.7.html Looking at the SmartOS manpage https://smartos.org/man/5/inotify there doesn't seem to be any equivalent language. What is the SmartOS requirement on the buffer size to be guaranteed to read at least one complete event ? Defining NAME_MAX to 255 seems like it shuts up the compiler error but does it give us the correct behaviour? thanks -- PMM
On 04.09.2017 19:50, Peter Maydell wrote: > On 4 September 2017 at 18:25, Kamil Rytarowski <n54@gmx.com> wrote: >> This fixes build on SmartOS (Joyent). >> >> Patch cherry-picked from pkgsrc by jperkin (Joyent). >> >> Signed-off-by: Kamil Rytarowski <n54@gmx.com> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> include/qemu/osdep.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h >> index 6855b94bbf..5d3860f80e 100644 >> --- a/include/qemu/osdep.h >> +++ b/include/qemu/osdep.h >> @@ -306,6 +306,11 @@ void qemu_anon_ram_free(void *ptr, size_t size); >> #endif >> #endif >> >> +/* Required by SmartOS (SunOS) */ >> +#ifndef NAME_MAX >> +#define NAME_MAX 255 >> +#endif > > So in hw/usb/dev-mtp.c we're using NAME_MAX in > char buf[sizeof(struct inotify_event) + NAME_MAX + 1]; > > because the Linux implementation of inotify documents in inotify(7) > that this is guaranteed to be sufficient to read at least one event: > http://man7.org/linux/man-pages/man7/inotify.7.html > Looking at the SmartOS manpage > https://smartos.org/man/5/inotify > there doesn't seem to be any equivalent language. > > What is the SmartOS requirement on the buffer size to be > guaranteed to read at least one complete event ? > Defining NAME_MAX to 255 seems like it shuts up the compiler > error but does it give us the correct behaviour? > According to my understanding SmartOS has the same logic. It tries to read at least one element, and NAME_MAX guarantees that it will be available. https://github.com/joyent/illumos-joyent/blob/master/usr/src/uts/common/io/inotify.c#L1193 I've gathered some overall details about the patches. 1. The inotify linux-compat interface is only SmartOS specific, it hasn't been upstreamed so far to other Illumos distributions. 2. Upstream Illumos implemented NAME_MAX https://github.com/illumos/illumos-gate/commit/9c0752ac0dc05794d2f8a8b4521d55e2b3f63247 It's not available in SmartOS. 3. Proprietary Solaris is out of scope of this work. Looking at their userland we can assume no qemu support: https://github.com/oracle/solaris-userland https://github.com/oracle/solaris-userland/commit/9c78c7b45a5d3dbd64afb455d278d2ca277e2b95#diff-94df904ebca88ee0ff038eaedb063ad6R61 ++ if platform.system() == 'SunOS': ++ # No QEMU support on Solaris now. ++ qemu_img = False We can stop pretending to support it now. 4. SmartOS forked qemu for its hypervisor part and uses qemu-kvm with the Linux kvm interface (yes, there is Linux kernel kvm port to SmartOS) - upstreaming the hypervisor's fork is out of scope. https://github.com/joyent/illumos-kvm-cmd 5. SmartOS ships with virtual machine guests, they host pkgsrc and qemu from pkgsrc. 6. Jonathan mentioned that preparing a SmartOS tutorial and image will be tricky, as SmartOS is a hypervisor. I will focus on upstreaming SmartOS-guest support for qemu, where there is pkgsrc. There is an option to prepare another Illumos distribution tutorial and testbot, hopefully it will be fully compatible with SmartOS patches. > thanks > -- PMM >
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 6855b94bbf..5d3860f80e 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -306,6 +306,11 @@ void qemu_anon_ram_free(void *ptr, size_t size); #endif #endif +/* Required by SmartOS (SunOS) */ +#ifndef NAME_MAX +#define NAME_MAX 255 +#endif + #if defined(__linux__) && \ (defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)) /* Use 2 MiB alignment so transparent hugepages can be used by KVM.