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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. > > https://bugzilla.redhat.com/show_bug.cgi?id=1412834 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/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1412834 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 > > https://bugs.launchpad.net/nova/+bug/1649527 -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1412834 > > 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/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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: [...] > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1412834 > > > > 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. >>>> >>>> https://bugzilla.redhat.com/show_bug.cgi?id=1412834 >> >> 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.