[v2] qemu: avoid slash characters to the new domain name.

[libvirt] [PATCH v2] qemu: avoid slash characters to the new domain name.

Posted by Julio Faracco 6 years ago

The 'domrename' command needs to check if the new domain name contains
the slash character. This character is not accepted by libvirt XML
definition because it is an invalid char (see Cole's commit b1fc6a7b7).
This commit enhace the 'domrename' command adding this check. The method
virDomainDefPostParse() was the other methods to check the domain are
not accessible.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/qemu/qemu_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5673d9fd8..9b16bc9c9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20773,6 +20773,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
     vm->def->name = new_dom_name;
     new_dom_name = NULL;
 
+    if (virDomainDefPostParse(vm->def, driver->caps, 0, driver->xmlopt, NULL) < 0)
+        goto rollback;
+
     if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
         goto rollback;
 
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] qemu: avoid slash characters to the new domain name.

Posted by Ján Tomko 6 years ago

On Mon, Apr 23, 2018 at 12:27:40AM -0300, Julio Faracco wrote:
>The 'domrename' command needs to check if the new domain name contains
>the slash character. This character is not accepted by libvirt XML
>definition because it is an invalid char (see Cole's commit b1fc6a7b7).
>This commit enhace the 'domrename' command adding this check. The method
>virDomainDefPostParse() was the other methods to check the domain are
>not accessible.
>
>Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
>---
> src/qemu/qemu_driver.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 5673d9fd8..9b16bc9c9 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -20773,6 +20773,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>     vm->def->name = new_dom_name;
>     new_dom_name = NULL;
>
>+    if (virDomainDefPostParse(vm->def, driver->caps, 0, driver->xmlopt, NULL) < 0)
>+        goto rollback;
>+

As its name suggests virDomainDefPostParse should be called after
parsing a domain definition from XML.

APIs doing small changes to domain definition should make sure that the
individual changes are okay, instead of invoking the full PostParse
machinery, which has the potential to change other stuff as well.

Jano

>     if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
>         goto rollback;
>
>-- 
>2.14.1
>
>--
>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

Re: [libvirt] [PATCH v2] qemu: avoid slash characters to the new domain name.

Posted by Julio Faracco 6 years ago

Hi Jan,

I totally agree. Using only virDomainDefPostParseCheckFeatures() is so
better than the full
virDomainDefPostParse() (as you mention too). But it is an internal
function which is not available
outside (for some reason).

Julio Cesar Faracco

2018-04-23 4:51 GMT-03:00 Ján Tomko <jtomko@redhat.com>:
> On Mon, Apr 23, 2018 at 12:27:40AM -0300, Julio Faracco wrote:
>>
>> The 'domrename' command needs to check if the new domain name contains
>> the slash character. This character is not accepted by libvirt XML
>> definition because it is an invalid char (see Cole's commit b1fc6a7b7).
>> This commit enhace the 'domrename' command adding this check. The method
>> virDomainDefPostParse() was the other methods to check the domain are
>> not accessible.
>>
>> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
>> ---
>> src/qemu/qemu_driver.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 5673d9fd8..9b16bc9c9 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -20773,6 +20773,9 @@ qemuDomainRenameCallback(virDomainObjPtr vm,
>>     vm->def->name = new_dom_name;
>>     new_dom_name = NULL;
>>
>> +    if (virDomainDefPostParse(vm->def, driver->caps, 0, driver->xmlopt,
>> NULL) < 0)
>> +        goto rollback;
>> +
>
>
> As its name suggests virDomainDefPostParse should be called after
> parsing a domain definition from XML.
>
> APIs doing small changes to domain definition should make sure that the
> individual changes are okay, instead of invoking the full PostParse
> machinery, which has the potential to change other stuff as well.
>
> Jano
>
>>     if (virDomainSaveConfig(cfg->configDir, driver->caps, vm->def) < 0)
>>         goto rollback;
>>
>> --
>> 2.14.1
>>
>> --
>> 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

Re: [libvirt] [PATCH v2] qemu: avoid slash characters to the new domain name.

Posted by Peter Krempa 6 years ago

[Please don't top-post on technical lists.]

On Mon, Apr 23, 2018 at 11:06:32 -0300, Julio Faracco wrote:
> Hi Jan,
> 
> I totally agree. Using only virDomainDefPostParseCheckFeatures() is so
> better than the full
> virDomainDefPostParse() (as you mention too). But it is an internal
> function which is not available
> outside (for some reason).

Well ... for the reason that it should not be used. Just do the check
here without any other checks. The only thing that is changed in this
API is the name.

I originally only objected to do the check in the virDomainRename
function. Doing it in qemuDomain rename is okay.

> 
> Julio Cesar Faracco
> 
> 2018-04-23 4:51 GMT-03:00 Ján Tomko <jtomko@redhat.com>:
> > On Mon, Apr 23, 2018 at 12:27:40AM -0300, Julio Faracco wrote:

[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list