https://bugzilla.redhat.com/show_bug.cgi?id=1607202
It's essentially stated in the nwfilterBindingDelete that we
will allow the admin to shoot themselves in the foot by deleting
the nwfilter binding which then allows them to undefine the
nwfilter that is in use for the running guest...
However, by allowing this we cause a problem for libvirtd
restart reconnect processing which would then try to recreate
the missing binding attempting to use the deleted filter
resulting in an error and thus shutting the guest down.
So rather than keep adding virDomainConfNWFilterInstantiate
flags to "ignore" specific error conditions, modify the logic
to ignore, but VIR_WARN errors other than ignoreExists. This
will at least allow the guest to not shutdown for only nwfilter
binding errors that we can now perhaps recover from since we
have the binding create/delete capability.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html
Differences to v2. Leave the ignoreExists bool, but just allow and
VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue
processing all filters from error point too.
src/qemu/qemu_process.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab749389ee..61a277f468 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def)
}
}
-static int
-qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
+/* Attempt to instantiate the filters. Ignore failures because it's
+ * possible that someone deleted a filter binding and the associated
+ * filter while the guest was running and we don't want that action
+ * to cause failure to keep the guest running during the reconnection
+ * processing. Nor do we necessarily want other failures to do the
+ * same. We'll just log the error conditions other than of course
+ * ignoreExists possibility (e.g. the true flag) */
+static void
+qemuProcessFiltersInstantiate(virDomainDefPtr def)
{
size_t i;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
if ((net->filter) && (net->ifname)) {
- if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0)
- return 1;
+ if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
+ true) < 0) {
+ VIR_WARN("filter '%s' instantiation for '%s' failed '%s'",
+ net->filter, net->ifname, virGetLastErrorMessage());
+ virResetLastError();
+ }
}
}
-
- return 0;
}
static int
@@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque)
qemuProcessNotifyNets(obj->def);
- if (qemuProcessFiltersInstantiate(obj->def, true))
- goto error;
+ qemuProcessFiltersInstantiate(obj->def);
if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
ping?
Tks,
John
On 08/24/2018 10:02 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1607202
>
> It's essentially stated in the nwfilterBindingDelete that we
> will allow the admin to shoot themselves in the foot by deleting
> the nwfilter binding which then allows them to undefine the
> nwfilter that is in use for the running guest...
>
> However, by allowing this we cause a problem for libvirtd
> restart reconnect processing which would then try to recreate
> the missing binding attempting to use the deleted filter
> resulting in an error and thus shutting the guest down.
>
> So rather than keep adding virDomainConfNWFilterInstantiate
> flags to "ignore" specific error conditions, modify the logic
> to ignore, but VIR_WARN errors other than ignoreExists. This
> will at least allow the guest to not shutdown for only nwfilter
> binding errors that we can now perhaps recover from since we
> have the binding create/delete capability.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>
> v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html
>
> Differences to v2. Leave the ignoreExists bool, but just allow and
> VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue
> processing all filters from error point too.
>
> src/qemu/qemu_process.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ab749389ee..61a277f468 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def)
> }
> }
>
> -static int
> -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
> +/* Attempt to instantiate the filters. Ignore failures because it's
> + * possible that someone deleted a filter binding and the associated
> + * filter while the guest was running and we don't want that action
> + * to cause failure to keep the guest running during the reconnection
> + * processing. Nor do we necessarily want other failures to do the
> + * same. We'll just log the error conditions other than of course
> + * ignoreExists possibility (e.g. the true flag) */
> +static void
> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
> {
> size_t i;
>
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
> if ((net->filter) && (net->ifname)) {
> - if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0)
> - return 1;
> + if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
> + true) < 0) {
> + VIR_WARN("filter '%s' instantiation for '%s' failed '%s'",
> + net->filter, net->ifname, virGetLastErrorMessage());
> + virResetLastError();
> + }
> }
> }
> -
> - return 0;
> }
>
> static int
> @@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque)
>
> qemuProcessNotifyNets(obj->def);
>
> - if (qemuProcessFiltersInstantiate(obj->def, true))
> - goto error;
> + qemuProcessFiltersInstantiate(obj->def);
>
> if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
> goto error;
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
ping^2?
Tks,
John
These are essentially what Daniel and I agreed upon during the v2 review
- link the patches below...
On 08/30/2018 06:24 PM, John Ferlan wrote:
> ping?
>
> Tks,
>
> John
>
> On 08/24/2018 10:02 AM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1607202
>>
>> It's essentially stated in the nwfilterBindingDelete that we
>> will allow the admin to shoot themselves in the foot by deleting
>> the nwfilter binding which then allows them to undefine the
>> nwfilter that is in use for the running guest...
>>
>> However, by allowing this we cause a problem for libvirtd
>> restart reconnect processing which would then try to recreate
>> the missing binding attempting to use the deleted filter
>> resulting in an error and thus shutting the guest down.
>>
>> So rather than keep adding virDomainConfNWFilterInstantiate
>> flags to "ignore" specific error conditions, modify the logic
>> to ignore, but VIR_WARN errors other than ignoreExists. This
>> will at least allow the guest to not shutdown for only nwfilter
>> binding errors that we can now perhaps recover from since we
>> have the binding create/delete capability.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>
>> v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html
>>
>> Differences to v2. Leave the ignoreExists bool, but just allow and
>> VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue
>> processing all filters from error point too.
>>
>> src/qemu/qemu_process.c | 24 ++++++++++++++++--------
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index ab749389ee..61a277f468 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def)
>> }
>> }
>>
>> -static int
>> -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
>> +/* Attempt to instantiate the filters. Ignore failures because it's
>> + * possible that someone deleted a filter binding and the associated
>> + * filter while the guest was running and we don't want that action
>> + * to cause failure to keep the guest running during the reconnection
>> + * processing. Nor do we necessarily want other failures to do the
>> + * same. We'll just log the error conditions other than of course
>> + * ignoreExists possibility (e.g. the true flag) */
>> +static void
>> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
>> {
>> size_t i;
>>
>> for (i = 0; i < def->nnets; i++) {
>> virDomainNetDefPtr net = def->nets[i];
>> if ((net->filter) && (net->ifname)) {
>> - if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0)
>> - return 1;
>> + if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
>> + true) < 0) {
>> + VIR_WARN("filter '%s' instantiation for '%s' failed '%s'",
>> + net->filter, net->ifname, virGetLastErrorMessage());
>> + virResetLastError();
>> + }
>> }
>> }
>> -
>> - return 0;
>> }
>>
>> static int
>> @@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque)
>>
>> qemuProcessNotifyNets(obj->def);
>>
>> - if (qemuProcessFiltersInstantiate(obj->def, true))
>> - goto error;
>> + qemuProcessFiltersInstantiate(obj->def);
>>
>> if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
>> goto error;
>>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
ping^3?
Tks
John
On 09/06/2018 07:03 PM, John Ferlan wrote:
>
> ping^2?
>
> Tks,
>
> John
>
> These are essentially what Daniel and I agreed upon during the v2 review
> - link the patches below...
>
> On 08/30/2018 06:24 PM, John Ferlan wrote:
>> ping?
>>
>> Tks,
>>
>> John
>>
>> On 08/24/2018 10:02 AM, John Ferlan wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1607202
>>>
>>> It's essentially stated in the nwfilterBindingDelete that we
>>> will allow the admin to shoot themselves in the foot by deleting
>>> the nwfilter binding which then allows them to undefine the
>>> nwfilter that is in use for the running guest...
>>>
>>> However, by allowing this we cause a problem for libvirtd
>>> restart reconnect processing which would then try to recreate
>>> the missing binding attempting to use the deleted filter
>>> resulting in an error and thus shutting the guest down.
>>>
>>> So rather than keep adding virDomainConfNWFilterInstantiate
>>> flags to "ignore" specific error conditions, modify the logic
>>> to ignore, but VIR_WARN errors other than ignoreExists. This
>>> will at least allow the guest to not shutdown for only nwfilter
>>> binding errors that we can now perhaps recover from since we
>>> have the binding create/delete capability.
>>>
>>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>>> ---
>>>
>>> v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html
>>>
>>> Differences to v2. Leave the ignoreExists bool, but just allow and
>>> VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue
>>> processing all filters from error point too.
>>>
>>> src/qemu/qemu_process.c | 24 ++++++++++++++++--------
>>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>> index ab749389ee..61a277f468 100644
>>> --- a/src/qemu/qemu_process.c
>>> +++ b/src/qemu/qemu_process.c
>>> @@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def)
>>> }
>>> }
>>>
>>> -static int
>>> -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
>>> +/* Attempt to instantiate the filters. Ignore failures because it's
>>> + * possible that someone deleted a filter binding and the associated
>>> + * filter while the guest was running and we don't want that action
>>> + * to cause failure to keep the guest running during the reconnection
>>> + * processing. Nor do we necessarily want other failures to do the
>>> + * same. We'll just log the error conditions other than of course
>>> + * ignoreExists possibility (e.g. the true flag) */
>>> +static void
>>> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
>>> {
>>> size_t i;
>>>
>>> for (i = 0; i < def->nnets; i++) {
>>> virDomainNetDefPtr net = def->nets[i];
>>> if ((net->filter) && (net->ifname)) {
>>> - if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net, ignoreExists) < 0)
>>> - return 1;
>>> + if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
>>> + true) < 0) {
>>> + VIR_WARN("filter '%s' instantiation for '%s' failed '%s'",
>>> + net->filter, net->ifname, virGetLastErrorMessage());
>>> + virResetLastError();
>>> + }
>>> }
>>> }
>>> -
>>> - return 0;
>>> }
>>>
>>> static int
>>> @@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque)
>>>
>>> qemuProcessNotifyNets(obj->def);
>>>
>>> - if (qemuProcessFiltersInstantiate(obj->def, true))
>>> - goto error;
>>> + qemuProcessFiltersInstantiate(obj->def);
>>>
>>> if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
>>> goto error;
>>>
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/24/2018 04:02 PM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1607202 > > It's essentially stated in the nwfilterBindingDelete that we > will allow the admin to shoot themselves in the foot by deleting > the nwfilter binding which then allows them to undefine the > nwfilter that is in use for the running guest... > > However, by allowing this we cause a problem for libvirtd > restart reconnect processing which would then try to recreate > the missing binding attempting to use the deleted filter > resulting in an error and thus shutting the guest down. > > So rather than keep adding virDomainConfNWFilterInstantiate > flags to "ignore" specific error conditions, modify the logic > to ignore, but VIR_WARN errors other than ignoreExists. This > will at least allow the guest to not shutdown for only nwfilter > binding errors that we can now perhaps recover from since we > have the binding create/delete capability. > > Signed-off-by: John Ferlan <jferlan@redhat.com> > --- > > v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html > > Differences to v2. Leave the ignoreExists bool, but just allow and > VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue > processing all filters from error point too. > > src/qemu/qemu_process.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.