On 2/27/20 12:01 PM, Stefan Reiter wrote:
> On 2/27/20 11:48 AM, Fabian Grünbichler wrote:
>> On February 26, 2020 11:53 am, Aaron Lauterer wrote:
>>> If IO-Thread is activated and a new enough Qemu version installed the
>>> program still ran into the elsif clause and never in the else clause.
>>> Thus the "include disk ..." log output was missing for these disks.
>>>
>>> Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
>>> ---
>>> PVE/VZDump/QemuServer.pm | 9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
>>> index 7695ad6..e36a259 100644
>>> --- a/PVE/VZDump/QemuServer.pm
>>> +++ b/PVE/VZDump/QemuServer.pm
>>> @@ -79,11 +79,12 @@ sub prepare {
>>> if (defined($drive->{backup}) && !$drive->{backup}) {
>>> $self->loginfo("exclude disk '$ds' '$volid' (backup=no)");
>>> return;
>>> - } elsif ($self->{vm_was_running} && $drive->{iothread}) {
>>> - if (!PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid,
>>> 4, 0, 1)) {
>>> - die "disk '$ds' '$volid' (iothread=on) can't use backup feature
>>> with running QEMU " .
>>> + } elsif ($self->{vm_was_running}
>>> + && $drive->{iothread}
>>> + && !PVE::QemuServer::Machine::runs_at_least_qemu_version($vmid,
>>> 4, 0, 1))
>> not new, I know, but:
>>
>> two of those three are not related to the drive. one of those two is
>> actually a qmp call. instead of having a condition spanning three lines,
>> we could refactor it like so:
>>
>> my $iothread_allowed = -1; // to avoid repeated QMP calls
>>
>> foreach_drive(..) {
>>
>> my $iothread = $self->{vm_was_running} && $drive->{iothread}; // do we even
>> care?
>> if ($iothread && $iothread_allowed == -1) {
>> $iothread_allowed =
>> PVE::QemuServer::Machine::runs_at_least_qemu_version(...); // cache on first
>> use
>> }
>>
>> if ( ... ) {
>>
>> } elsif ($iothread && !$iothread_allowed) { // simple condition ;)
>> die ...
>> }
>>
>> alternatively, the die could also move into the else branch, assuming we
>> don't have efidisks with iothreads?
>>
>> also, isn't this actually wrong? in stop mode, we'd get the new qemu
>> version when it counts, even if the VM is running the old version right
>> now?
>>
>
> Doesn't a "stop" mode backup imply the VM being shut down and restarted
> before backup? I.e. the backup always will run on the currently installed
> QEMU version?
>
> If that's the case, this is correct, since we only care about which QEMU
> version we send the QMP command to.
>
what's the outcome/state of this?
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
