Bug#895898: [Pkg-libvirt-maintainers] Bug#895898: virt-install adds "method=" boot parameter, which is copied to Grub config

2018-04-25 Thread Raphaël Halimi
Le 18/04/2018 à 19:13, Guido Günther a écrit :
> That's a good start. Please forward this upstream (the actual fix will
> likely have to be in a slightly different location)

Hi,

According to [1], the bug was already was already fixed upstream (but
not in version 1.5 (which is still to be packaged), after a look at [2];
probably for a future release).

I'll let to your appreciation what to do with my small patch, packaging
it in unstable (or even stable, if you plan to backport 1.4.3 or 1.5
someday)  would be nice, but if you don't want to bother with a bug
which will eventually be fixed upstream after 1.5, I'd understand.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1569485#c1
[2]
https://github.com/virt-manager/virt-manager/blob/40c678783e6c7d1f1c5c9b09e2640a54eb8be5a4/virtinst/urlfetcher.py

Regards,

-- 
Raphaël Halimi



signature.asc
Description: OpenPGP digital signature


Bug#895898: [Pkg-libvirt-maintainers] Bug#895898: virt-install adds "method=" boot parameter, which is copied to Grub config

2018-04-18 Thread Guido Günther
Hi,
On Wed, Apr 18, 2018 at 05:36:13PM +0200, Raphaël Halimi wrote:
> Le 18/04/2018 à 08:45, Guido Günther a écrit :
> > Check _kernelFetchHelper where it adds the method arg. I doubt this is
> > needed at all on Debian but rather a leftover of other distros.
> 
> Hi again,
> 
> Thanks to your hint, and after a bit of fiddling (remember I don't know
> python), I came up with the simple patch attached.
> 
> It implements the second solution (remove "method=" altogether if the
> distribution is detected as "Debian").
> 
> It should be tested with other guest distributions though.
> 
> Also, I think you could also check if self.name is equal to "Ubuntu",
> but I'm not sure if ubuntu-installer needs (or support) "method=" or not.
> 
> Do you want me to create and forward a bug upstream ?

That's a good start. Please forward this upstream (the actual fix will
likely have to be in a slightly different location)
Cheers
 -- Guido

> 
> Regards,
> 
> -- 
> Raphaël Halimi

> --- /usr/share/virt-manager/virtinst/urlfetcher.py.orig   2017-09-14 
> 23:49:00.0 +0200
> +++ /usr/share/virt-manager/virtinst/urlfetcher.py2018-04-18 
> 17:29:08.902639187 +0200
> @@ -661,7 +661,8 @@
>  args = ''
>  
>  if not self.fetcher.location.startswith("/"):
> -args += "%s=%s" % (self._get_method_arg(), self.fetcher.location)
> +if self.name != "Debian":
> +args += "%s=%s" % (self._get_method_arg(), 
> self.fetcher.location)



Bug#895898: [Pkg-libvirt-maintainers] Bug#895898: virt-install adds "method=" boot parameter, which is copied to Grub config

2018-04-18 Thread Raphaël Halimi
Le 18/04/2018 à 08:45, Guido Günther a écrit :
> Check _kernelFetchHelper where it adds the method arg. I doubt this is
> needed at all on Debian but rather a leftover of other distros.

Hi again,

Thanks to your hint, and after a bit of fiddling (remember I don't know
python), I came up with the simple patch attached.

It implements the second solution (remove "method=" altogether if the
distribution is detected as "Debian").

It should be tested with other guest distributions though.

Also, I think you could also check if self.name is equal to "Ubuntu",
but I'm not sure if ubuntu-installer needs (or support) "method=" or not.

Do you want me to create and forward a bug upstream ?

Regards,

-- 
Raphaël Halimi
--- /usr/share/virt-manager/virtinst/urlfetcher.py.orig	2017-09-14 23:49:00.0 +0200
+++ /usr/share/virt-manager/virtinst/urlfetcher.py	2018-04-18 17:29:08.902639187 +0200
@@ -661,7 +661,8 @@
 args = ''
 
 if not self.fetcher.location.startswith("/"):
-args += "%s=%s" % (self._get_method_arg(), self.fetcher.location)
+if self.name != "Debian":
+args += "%s=%s" % (self._get_method_arg(), self.fetcher.location)
 
 try:
 initrd = self.fetcher.acquireFile(initrdpath)


signature.asc
Description: OpenPGP digital signature


Bug#895898: [Pkg-libvirt-maintainers] Bug#895898: virt-install adds "method=" boot parameter, which is copied to Grub config

2018-04-18 Thread Guido Günther
Hi,
On Tue, Apr 17, 2018 at 01:55:33PM +0200, Raphaël Halimi wrote:
> Package: virtinst
> Version: 1:1.4.0-5
> Severity: minor
> 
> Hi,
> 
> I'm trying to fully automatically install virtual machines by means of a
> preseed file. I set "--location" to the desired URL and add some
> parameters to "--extra-args" to ask debian-installer to run an automated
> installation, tell it where to fetch the preseed file, and add some
> parameters that I want copied to the installed system's boot
> configuration (for example "elevator=noop").
> 
> This works almost perfectly, but there one slight problem: for a reason
> I don't understand, virt-install adds "method= option>" to the kernel parameters (qemu's "-append" option), *after* the
> contents of "--extra-args".
> 
> As stated in its documentation, debian-installer will copy most kernel
> parameters found after "--" or "---" to the installed system's boot
> configuration (eg. in /etc/default/grub), filtering the ones it thinks
> are destined to itself; so if the "--extra-args" option contains "--" or
> "---", the "method=..." argument ends up being copied in
> /etc/default/grub, which of course is not desired. Obviously,
> debian-installer filter doesn't catch it.
> 
> Now, I'm not sure what would be the correct fix for this. I see two
> possibilities:
> 
> - the conservative (and simple) one: modify virt-install to add
> "method=..." *before* the contents of "--extra-args", so that if the
> latter contains "--" or "---", "method=..." will be positioned before
> those, and won't be copied to the installed system's boot configuration.
> 
> - the risky one: modify virt-install to not add "method=..." at all;
> after re-reading debian-installer's documentation, I didn't see this
> parameter mentioned anywhere. Come to think of it, this shouldn't be
> needed: virt-install already fetches the kernel and initrd, and passes
> them to the VM, so I can't see why debian-installer should need to know
> how they were fetched.
> 
> If the second suggestion is totally wrong, and debian-installer does
> know (and thus, sometimes need) "method=..." (it should be documented,
> then !), maybe the bug should be reassigned to debian-installer, and ask
> to add "method=..." to the list of filtered parameters, to prevent it to
> be passed to Grub.
> 
> I tried to find by myself where the "-append" option is constructed, to
> provide a patch for the first fix, but I don't know much python and
> couldn't understand the scripts. If you give me a hint about this, I'll
> be glad to (try to) help.
> 

Check _kernelFetchHelper where it adds the method arg. I doubt this is
needed at all on Debian but rather a leftover of other distros.
 -- Guido

> Regards,
> 
> -- 
> Raphaël Halimi
> 




> ___
> Pkg-libvirt-maintainers mailing list
> pkg-libvirt-maintain...@alioth-lists.debian.net
> https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-libvirt-maintainers