Re: [libvirt PATCH v3 0/8] vmx: Don't error out on missing filename for cdrom

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Tue, Jan 05, 2021 at 07:19:08PM +0100, Michal Privoznik wrote:
> On 1/5/21 4:54 PM, Martin Kletzander wrote:
> > This is perfectly valid in VMWare and the VM just boots with an empty drive.  We
> > used to just skip the whole drive before, but since we changed how we parse
> > empty cdrom drives this now results in an error and the user not being able to
> > even dump the XML.  Instead of erroring out, just keep the drive empty.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953
> >
> > v3:
> >   - Fixed the vmware driver
> >   - Bit of a clean-up
> >   - Few more tests
> >
> > v2:
> >   - Do not report and reset an error, but handle it more nicely.
> >   - https://www.redhat.com/archives/libvir-list/2020-December/msg00846.html
> >
> > v1:
> >   - https://www.redhat.com/archives/libvir-list/2020-December/msg00840.html
> >
> > Martin Kletzander (8):
> >    esx: Unindent unnecessary conditional branch
> >    tests: Use g_autofree in testParseVMXFileName
> >    vmx: Make virVMXParseFileName return an integer
> >    tests: Allow testing for parse failures in vmx2xmltest
> >    vmx: Allow missing cdrom image file in virVMXParseFileName
> >    tests: Test vmx files with missing images
> >    esx: Handle missing images in esxParseVMXFileName
> >    vmx: Treat missing cdrom-image as empty drive
> >
> >   src/esx/esx_driver.c                          | 160 ++++++++++--------
> >   src/vmware/vmware_conf.c                      |  21 ++-
> >   src/vmware/vmware_conf.h                      |  10 +-
> >   src/vmware/vmware_driver.c                    |   6 +-
> >   src/vmx/vmx.c                                 |  27 +--
> >   src/vmx/vmx.h                                 |   5 +-
> >   ...x2xml-cdrom-ide-file-missing-datastore.vmx |   6 +
> >   .../vmx2xml-cdrom-ide-file-missing-file.vmx   |   6 +
> >   ...ml-harddisk-ide-file-missing-datastore.vmx |   6 +
> >   ...mx2xml-harddisk-scsi-file-missing-file.vmx |   7 +
> >   tests/vmx2xmltest.c                           |  67 +++++---
> >   11 files changed, 203 insertions(+), 118 deletions(-)
> >   create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
> >   create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
> >   create mode 100644 tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
> >   create mode 100644 tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx
>
> Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>

Thanks, although I *again* forgot to add your R-b to the patches...  I wish
gitlab took care of that.  And I even have a function and a command for it:

function rb --description 'Add Reviewed-by: to current commit' --argument who
    set git_cmd (command -s git)
    if ! test $git_cmd
        begin
            set_color red
            echo -n "error: "
            set_color normal
            echo "Command `git` not available"
        end >&2
        return 1
    end
    if ! test $who
        begin
            set_color red
            echo -n "error: "
            set_color normal
            echo "Missing parameter `who`"
        end >&2
        return 1
    end
    env VISUAL='git interpret-trailers --in-place --trailer "Reviewed-by='$who'"' $git_cmd commit --amend
end

Anyway, thanks, and sorry O:-)

> Michal

Attachment: signature.asc
Description: PGP signature