[libvirt] [PATCH 0/2] vz: build fix
Diff from v1: - add patch "check scripts: handle unintialized driver vars in check-driverimpls.pl" Nikolay Shirokovskiy (2): check scripts: handle unintialized driver vars in check-driverimpls.pl vz: build fix src/check-driverimpls.pl | 2 +- src/vz/vz_driver.c | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) -- 1.8.3.1
Current script does not recognize uninitialized driver vars and interprets next lines as if there is initialization section. Let's assume we have sane initialization syntax for drivers if it's present and check we if we have open braces. --- src/check-driverimpls.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 4c28f20..63da2e1 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -67,7 +67,7 @@ while (<>) { $status = 1; } } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { + } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) { next if $1 eq "virNWFilterCallbackDriver" || $1 eq "virNWFilterTechDriver" || $1 eq "virConnectDriver"; -- 1.8.3.1
On Wed, Apr 18, 2018 at 05:31:12PM +0300, Nikolay Shirokovskiy wrote:
Current script does not recognize uninitialized driver vars and interprets next lines as if there is initialization section.
IIUC, the problem is the line static virHypervisorDriver parallelsHypervisorDriver; confuses it because it doesn't start a block ?
Let's assume we have sane initialization syntax for drivers if it's present and check we if we have open braces. --- src/check-driverimpls.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 4c28f20..63da2e1 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -67,7 +67,7 @@ while (<>) { $status = 1; } } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { + } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) {
I'd rather we did a blacklist of the ";" character, so we still detect it if there's other unusal formatting used eg use a negative assertion lookahead like } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) { Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 18.04.2018 20:25, Daniel P. Berrangé wrote:
On Wed, Apr 18, 2018 at 05:31:12PM +0300, Nikolay Shirokovskiy wrote:
Current script does not recognize uninitialized driver vars and interprets next lines as if there is initialization section.
IIUC, the problem is the line
static virHypervisorDriver parallelsHypervisorDriver;
confuses it because it doesn't start a block ?
Yes.
Let's assume we have sane initialization syntax for drivers if it's present and check we if we have open braces. --- src/check-driverimpls.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 4c28f20..63da2e1 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -67,7 +67,7 @@ while (<>) { $status = 1; } } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { + } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) {
I'd rather we did a blacklist of the ";" character, so we still detect it if there's other unusal formatting used eg use a negative assertion lookahead like
} elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) {
Agree. With your patch in case of misformatting ; we can break and fix formatting. With my patch in case of misformatting { we can skip the check which is worse. I'll push series with your patch if you don't mind. Nikolay
On Thu, Apr 19, 2018 at 09:56:07AM +0300, Nikolay Shirokovskiy wrote:
On 18.04.2018 20:25, Daniel P. Berrangé wrote:
On Wed, Apr 18, 2018 at 05:31:12PM +0300, Nikolay Shirokovskiy wrote:
Current script does not recognize uninitialized driver vars and interprets next lines as if there is initialization section.
IIUC, the problem is the line
static virHypervisorDriver parallelsHypervisorDriver;
confuses it because it doesn't start a block ?
Yes.
Let's assume we have sane initialization syntax for drivers if it's present and check we if we have open braces. --- src/check-driverimpls.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 4c28f20..63da2e1 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -67,7 +67,7 @@ while (<>) { $status = 1; } } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { + } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) {
I'd rather we did a blacklist of the ";" character, so we still detect it if there's other unusal formatting used eg use a negative assertion lookahead like
} elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) {
Agree. With your patch in case of misformatting ; we can break and fix formatting. With my patch in case of misformatting { we can skip the check which is worse.
I'll push series with your patch if you don't mind.
Sure that's fine. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 19.04.2018 10:49, Daniel P. Berrangé wrote:
On Thu, Apr 19, 2018 at 09:56:07AM +0300, Nikolay Shirokovskiy wrote:
On 18.04.2018 20:25, Daniel P. Berrangé wrote:
On Wed, Apr 18, 2018 at 05:31:12PM +0300, Nikolay Shirokovskiy wrote:
Current script does not recognize uninitialized driver vars and interprets next lines as if there is initialization section.
IIUC, the problem is the line
static virHypervisorDriver parallelsHypervisorDriver;
confuses it because it doesn't start a block ?
Yes.
Let's assume we have sane initialization syntax for drivers if it's present and check we if we have open braces. --- src/check-driverimpls.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl index 4c28f20..63da2e1 100755 --- a/src/check-driverimpls.pl +++ b/src/check-driverimpls.pl @@ -67,7 +67,7 @@ while (<>) { $status = 1; } } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { + } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) {
I'd rather we did a blacklist of the ";" character, so we still detect it if there's other unusal formatting used eg use a negative assertion lookahead like
} elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) {
Agree. With your patch in case of misformatting ; we can break and fix formatting. With my patch in case of misformatting { we can skip the check which is worse.
I'll push series with your patch if you don't mind.
Sure that's fine.
Thanx! Pushed. Nikolay
Broken by [1] commit - trailing comma instead of semicolon. Fortunately the issue did not get sneak in released 4.2 version. Note that uriSchemes for parallelsConnectDriver should not be allocated on stack. [1] 8e4f9a27: "driver: declare supported URI schemes in virConnectDriver struct" --- src/vz/vz_driver.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 4c30ee1..a9ee773 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -4163,7 +4163,11 @@ static virStateDriver vzStateDriver = { /* Parallels domain type backward compatibility*/ static virHypervisorDriver parallelsHypervisorDriver; -static virConnectDriver parallelsConnectDriver; +static virConnectDriver parallelsConnectDriver = { + .localOnly = true, + .uriSchemes = (const char *[]){ "parallels", NULL }, + .hypervisorDriver = ¶llelsHypervisorDriver, +}; /** * vzRegister: @@ -4186,9 +4190,6 @@ vzRegister(void) /* Backward compatibility with Parallels domain type */ parallelsHypervisorDriver = vzHypervisorDriver; parallelsHypervisorDriver.name = "Parallels"; - parallelsConnectDriver = vzConnectDriver; - parallelsConnectDriver.hypervisorDriver = ¶llelsHypervisorDriver; - parallelsConnectDriver.uriSchemes = (const char *[]){ "parallels", NULL }, if (virRegisterConnectDriver(¶llelsConnectDriver, true) < 0) return -1; -- 1.8.3.1
participants (2)
-
Daniel P. Berrangé -
Nikolay Shirokovskiy