[Pkg-xen-devel] Xen 4.1.1 packaging enhancement

[Ian Campbell]

On Thu, 2011-08-18 at 17:53 +0800, Thomas Goirand wrote:
> >>  Description: Xenstore communications library for Xen
> >> + XenStore is a database, hosted by domain 0,
> > 
> > Not necessarily dom0 -- it is possible to run xenstore in another domain
> > (although only really prototypes of this have been done). </pendant>
> 
> Can you provide a better wording then? Because currently, we have a
> single line long description, which isn't good for neither my taste and
> the one of Lintian. :)

Just drop the ", hosted by domain 0," ?

> >> +install-lib-ocaml_$(ARCH): DIR = $(BUILD_DIR)/install-utils_$(ARCH)
> >> +install-lib-ocaml_$(ARCH): PACKAGE_NAME = libxen-ocaml
> >> +install-lib-ocaml_$(ARCH): DH_OPTIONS = -p$(PACKAGE_NAME)
> >> +install-lib-ocaml_$(ARCH): $(STAMPS_DIR)/install-utils_$(ARCH)
> >> +	dh_testdir
> >> +	dh_testroot
> >> +	dh_prep
> >> +	install -D -m 0644 $(BUILD_DIR)/install-utils_$(ARCH)/usr/lib/ocaml/eventchn/dlleventchn_stubs.so $(CURDIR)/debian/$(PACKAGE_NAME)/$(OCAML_DLL_DIR)/dlleventchn_stubs.so
> >> +	install -D -m 0644 $(BUILD_DIR)/install-utils_$(ARCH)/usr/lib/ocaml/mmap/dllmmap_stubs.so $(CURDIR)/debian/$(PACKAGE_NAME)/$(OCAML_DLL_DIR)/dllmmap_stubs.so
> >> +	install -D -m 0644 $(BUILD_DIR)/install-utils_$(ARCH)/usr/lib/ocaml/xb/dllxb_stubs.so $(CURDIR)/debian/$(PACKAGE_NAME)/$(OCAML_DLL_DIR)/dllxb_stubs.so
> >> +	install -D -m 0644 $(BUILD_DIR)/install-utils_$(ARCH)/usr/lib/ocaml/xc/dllxc_stubs.so $(CURDIR)/debian/$(PACKAGE_NAME)/$(OCAML_DLL_DIR)/dllxc_stubs.so
> >> +	install -D -m 0644 $(BUILD_DIR)/install-utils_$(ARCH)/usr/lib/ocaml/xl/dllxl_stubs.so $(CURDIR)/debian/$(PACKAGE_NAME)/$(OCAML_DLL_DIR)/dllxl_stubs.so
> > 
> > All these open-coded install lines (and the many which follow) can't be
> > right. Why can't dh_install be used for this stuff?
> 
> The point is to use $(OCAML_DLL_DIR) from /usr/share/ocaml/ocamlvars.mk,
> which dh_install can't use. But it's wrong as well, best would be to use
> "ocamlfind install". I'll try working on that.

Does OCAML_DDL_DIR need plumbing into a (potentially new) upstream
configuration option so that the "make -C tools" Just Works? Or does
"ocamlfind install" already just work (I thought upstream was using it
already though).

> 
> >> @@ -157,6 +228,7 @@
> >>  	install -D -m644 debian/xen-utils.README.Debian $(PACKAGE_DIR)/usr/share/doc/$(PACKAGE_NAME)/README.Debian
> >>  	install -D -m644 debian/xen-utils-$(VERSION).lintian-overrides $(PACKAGE_DIR)/usr/share/lintian/overrides/$(PACKAGE_NAME)
> >>  	dh_install --sourcedir=$(DIR) usr/lib/xen-$(VERSION)
> >> +	strip -s --remove-section=.note --remove-section=.comment $(PACKAGE_DIR)/usr/lib/xen-$(VERSION)/boot/hvmloader
> > 
> > What is this for?
> 
> Isn't that obvious? Stripping hvmloader which was *not* stripped as
> reported by lintian... I was expecting that dh_stirp would do it, but
> it's not. It's a bit awkward to do it this way, but it works! :)

Ah, I thought you were specifically removing .note and .comment over and
above what dh_strip had done.

I suppose dh_strip wants/needs it to be executable in order to find it
or only looks in *bin or something. It's a shame dh_strip can't take a
list of additional files from somewhere

This isn't really a "binary" in the sense which I think policy 10.1
means (it just happens to be an ELF file). I think overriding the
Lintian warning would have been an appropriate response also.

> 
> >> diff -r -u old/xen-4.1.1/qemu/audio/alsaaudio.c new/xen-4.1.1/qemu/audio/alsaaudio.c
> >> --- old/xen-4.1.1/qemu/audio/alsaaudio.c	2011-04-28 07:38:36.000000000 +0000
> >> +++ new/xen-4.1.1/qemu/audio/alsaaudio.c	2011-08-07 07:40:06.000000000 +0000
> >> @@ -475,7 +475,7 @@
> >>          (obt->fmt != req->fmt ||
> >>           obt->nchannels != req->nchannels ||
> >>           obt->freq != req->freq)) {
> >> -        dolog ("Audio paramters for %s\n", typ);
> >> +        dolog ("Audio parameters for %s\n", typ);
> >>          alsa_dump_info (req, obt);
> >>      }
> >>
> > 
> > Should this (and all the following changes to the qemu subtree) be
> > patches under debian/patches instead of applied directly?
> 
> Same as above. It WAS patched in debian/patches, but I didn't add the -N
> option to diff, and new files didn't show up.

It'd be useful to exclude these bits of the patch in favour of including
debian/patches, just to avoid confusion/duplication etc.

> 
> >> +LIBS += -lpulse -lSDL
> > 
> > Should rather be a fix to the configure script?
> > 
> >>  BAD_OBJS += gdbstub.o acpi.o apic.o
> >>  BAD_OBJS += vmmouse.o vmport.o tcg* helper.o vmware_vga.o virtio-balloon.o
> >>  
> >> diff -r -u old/xen-4.1.1/tools/Makefile new/xen-4.1.1/tools/Makefile
> >> --- old/xen-4.1.1/tools/Makefile	2011-08-07 01:02:50.000000000 +0000
> >> +++ new/xen-4.1.1/tools/Makefile	2011-08-07 07:40:06.000000000 +0000
> >> @@ -109,7 +109,7 @@
> >>  		$(absolutify_xen_root); \
> >>  		$(buildmakevars2shellvars); \
> >>  		cd ioemu-dir; \
> >> -		$(QEMU_ROOT)/xen-setup $(IOEMU_CONFIGURE_CROSS)
> >> +		$(QEMU_ROOT)/xen-setup $(IOEMU_CONFIGURE_CROSS) --audio-drv-list="pa oss alsa sdl esd" --audio-card-list="ac97 es1370 sb16 cs4231a adlib gus" --enable-mixemu
> > 
> > It'd be better to add a IOEMU_CONFIG_OPTS variable to the top-level
> > Config.mk and use it in the packaging. That sort of patch could be
> > upstreamed I think.
> > 
> > Ian.
> 
> Can you explain better how to do that? Really, we do need the audio and
> SDL options, so it'd be nice to get to know how to do it the best way.

e.g. add to tools/Makefile:
	IOEMU_CONFIGURE_OPTIONS ?=
and make the above xen-setup invocation:
	$(QEMU_ROOT)/xen-setup $(IOEMU_CONFIGURE_CROSS) $(IOEMU_CONFIGURE_OPTIONS)
and getting that patch upstream would allow you to define
IOEMU_CONFIGURE_OPTIONS in the packaging files instead of having to
patch.

Ian.

-- 
Ian Campbell
Current Noise: Soundgarden - Black Hole Sun

Playing an unamplified electric guitar is like strumming on a picnic table.
		-- Dave Barry, "The Snake"