Message ID | 6B17869C-8BD8-4C5F-AF64-74E704255A2B@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Christian Lindig writes ("Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD"): > This approach adds two new entries into oxenstored.conf that are > determined by the configure script. I prefer it over the previous > design not the least because it results in a much smaller change and > doesn’t require new bindings for C libraries. The code looks good > and I believe it doesn’t change in significant ways in its failure > modes as port and fd are still read from files - just the names of > these files are now coming from oxenstored.conf. I’m not sure the > paths.m4 is taking the best approach by relying on $host_os rather > than testing the paths but I would leave that to people with more > autoconf experience to comment on. Let me repeat something I said in the corridor: we mustn't test for the existence of these paths at build-time, because the build host might not be running Xen. Personally I would have just tested both paths at runtime, but this configure-based approach is fine too. > +case "$host_os" in > +*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;; > +*) XENSTORED_KVA=/proc/xen/xsd_kva ;; > +esac > +AC_SUBST(XENSTORED_KVA) > + > +case "$host_os" in > +*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;; > +*) XENSTORED_PORT=/proc/xen/xsd_port ;; > +esac > +AC_SUBST(XENSTORED_PORT) I'm not sure why one wouldn't combine these two case statements, but I don't care enough to suggest reworking it. All three patches: Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Thanks, Ian.
Hi, On 20/04/17 11:22, Ian Jackson wrote: > Christian Lindig writes ("Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD"): >> This approach adds two new entries into oxenstored.conf that are >> determined by the configure script. I prefer it over the previous >> design not the least because it results in a much smaller change and >> doesn’t require new bindings for C libraries. The code looks good >> and I believe it doesn’t change in significant ways in its failure >> modes as port and fd are still read from files - just the names of >> these files are now coming from oxenstored.conf. I’m not sure the >> paths.m4 is taking the best approach by relying on $host_os rather >> than testing the paths but I would leave that to people with more >> autoconf experience to comment on. > > Let me repeat something I said in the corridor: we mustn't test for > the existence of these paths at build-time, because the build host > might not be running Xen. > > Personally I would have just tested both paths at runtime, but this > configure-based approach is fine too. > >> +case "$host_os" in >> +*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;; >> +*) XENSTORED_KVA=/proc/xen/xsd_kva ;; >> +esac >> +AC_SUBST(XENSTORED_KVA) >> + >> +case "$host_os" in >> +*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;; >> +*) XENSTORED_PORT=/proc/xen/xsd_port ;; >> +esac >> +AC_SUBST(XENSTORED_PORT) > > I'm not sure why one wouldn't combine these two case statements, but I > don't care enough to suggest reworking it. > > All three patches: > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Release-acked-by: Julien Grall <julien.grall@arm.com> Cheers,
diff --git a/m4/paths.m4 b/m4/paths.m4 index 93ce89ab40..f208b7e39f 100644 --- a/m4/paths.m4 +++ b/m4/paths.m4 @@ -147,3 +147,15 @@ AC_SUBST(XEN_PAGING_DIR) XEN_DUMP_DIR=$xen_dumpdir_path AC_SUBST(XEN_DUMP_DIR) ]) + +case "$host_os" in +*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;; +*) XENSTORED_KVA=/proc/xen/xsd_kva ;; +esac +AC_SUBST(XENSTORED_KVA) + +case "$host_os" in +*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;; +*) XENSTORED_PORT=/proc/xen/xsd_port ;; +esac +AC_SUBST(XENSTORED_PORT)