[libvirt] [PATCH] qemu: Allow empty script path to <interface/>
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration: <interface type='ethernet'/> <script path=''/> </interface> This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 10 ++++++++++ src/util/virnetdev.c | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index f408293a1..e7388a8f6 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -56,6 +56,16 @@ a fabric name has been removed by making it optional. </description> </change> + <change> + <summary> + qemu: Allow empty script path to <interface/> + </summary> + <description> + Historically, this was always allowed. Unfortunately, due to some + rework done for 1.3.2 release a bug was dragged in which suddenly + stop allowing domain with such configuration to start. + </description> + </change> </section> </release> <release version="v3.0.0" date="2017-01-17"> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index fa695d4a6..d12324878 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2615,6 +2615,10 @@ virNetDevRunEthernetScript(const char *ifname, const char *script) virCommandPtr cmd; int ret; + /* Not a bug! Previously we did accept script="" as a NO-OP. */ + if (STREQ(script, "")) + return 0; + cmd = virCommandNew(script); virCommandAddArgFormat(cmd, "%s", ifname); virCommandClearCaps(cmd); -- 2.11.0
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On 02/02/2017 02:56 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this.
Fair enough. Although, 1.3.2 was not that long time ago (~a year). BTW: it's openstack who is still using this. https://bugzilla.redhat.com/show_bug.cgi?id=1412834 Michal
On Thu, Feb 02, 2017 at 04:21:39PM +0100, Michal Privoznik wrote:
On 02/02/2017 02:56 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this.
Fair enough. Although, 1.3.2 was not that long time ago (~a year). BTW: it's openstack who is still using this.
Seems nova has already been fixed in both master & stable branches https://bugs.launchpad.net/nova/+bug/1649527 Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On Thu, Feb 02, 2017 at 03:29:15PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 04:21:39PM +0100, Michal Privoznik wrote:
On 02/02/2017 02:56 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this.
Fair enough. Although, 1.3.2 was not that long time ago (~a year). BTW: it's openstack who is still using this.
IIUC, this bug was filed to handle the case of _existing_ VMs that were not launched with the patch that you point to below.
Seems nova has already been fixed in both master & stable branches
-- /kashyap
On Thu, Feb 02, 2017 at 04:55:29PM +0100, Kashyap Chamarthy wrote:
On Thu, Feb 02, 2017 at 03:29:15PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 04:21:39PM +0100, Michal Privoznik wrote:
On 02/02/2017 02:56 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this.
Fair enough. Although, 1.3.2 was not that long time ago (~a year). BTW: it's openstack who is still using this.
IIUC, this bug was filed to handle the case of _existing_ VMs that were not launched with the patch that you point to below.
THat is only a problem if you have a running VM on libvirt < 1.3.2 and you want to migrate it to libvirt >= 1.3.2. That can be solved by removing the bogus script element prior to migration - nova already has code that modifies the XML prior to migration where such a fix could live Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On Thu, Feb 02, 2017 at 05:12:54PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 04:55:29PM +0100, Kashyap Chamarthy wrote:
[...]
IIUC, this bug was filed to handle the case of _existing_ VMs that were not launched with the patch that you point to below.
THat is only a problem if you have a running VM on libvirt < 1.3.2 and you want to migrate it to libvirt >= 1.3.2. That can be solved by removing the bogus script element prior to migration - nova already has code that modifies the XML prior to migration where such a fix could live
Ah, that makes sense, the bug can then be closed. Thanks for clearly spelling out the only problem scenario. -- /kashyap
On 02/02/2017 06:12 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 04:55:29PM +0100, Kashyap Chamarthy wrote:
On Thu, Feb 02, 2017 at 03:29:15PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 04:21:39PM +0100, Michal Privoznik wrote:
On 02/02/2017 02:56 PM, Daniel P. Berrange wrote:
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This was always invalid input and the fact that it happened to work in the past is just luck. Since this has been broken since 1.3.2 there's plenty of libvirt releases that reject this, and so any app realistically has to be fixed to omit the <script> entirely if they want any portability. As such I don't think we need to "fix" this.
Fair enough. Although, 1.3.2 was not that long time ago (~a year). BTW: it's openstack who is still using this.
IIUC, this bug was filed to handle the case of _existing_ VMs that were not launched with the patch that you point to below.
THat is only a problem if you have a running VM on libvirt < 1.3.2 and you want to migrate it to libvirt >= 1.3.2. That can be solved by removing the bogus script element prior to migration - nova already has code that modifies the XML prior to migration where such a fix could live
Question is - does it live there already? If not, we should fix it there too. Michal
On Thu, Feb 02, 2017 at 02:49:48PM +0100, Michal Privoznik wrote:
Before 9c17d665fdc5f (v1.3.2 - I know, right?) it was possible to have the following interface configuration:
<interface type='ethernet'/> <script path=''/> </interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped us to get away with this as it just ignored the empty script path. However, after the commit mentioned above it's libvirtd who is executing the script. Unfortunately without special case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 10 ++++++++++ src/util/virnetdev.c | 4 ++++ 2 files changed, 14 insertions(+)
ACK because this causes upgrade problems when using migration. Applications like OpenStack use migration as their upgrade mechanism. ie to upgrade a new version of Nova, you would install it on a new compute host and migrate your guests across. For nova to fix its buggy usage, users would have to upgrade Nova in-place on existing nodes which is contrary to its normal practice of using migration for all upgradess Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
participants (3)
-
Daniel P. Berrange -
Kashyap Chamarthy -
Michal Privoznik