Re: [PATCHv3 2/4] VMX: Add cdrom-raw dev type from VMWare Fusion

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On Thu, Aug 29, 2013 at 5:19 AM, Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:

On 28.08.2013 23:53, Doug Goldstein wrote:
> According to VMWare's documentation 'cdrom-raw' is an acceptable value
> for deviceType for a CD-ROM drive. The documentation states that the VMX
> configuration for a CD-ROM deviceType is as follows:
>
> ide|scsi(n):(n).deviceType = "cdrom-raw|atapi-cdrom|cdrom-image"
>
>>From the documentation it appears the following is true:

s/>//

I'm not understanding what you're seeing. I don't see an extra > anywhere.

 

> - cdrom-image = Provides the ISO to the VM
> - atapi-cdrom = Provides a NEC emulated ATAPI CD-ROM on top of the host
>   CD-ROM
> - cdrom-raw = Passthru for a host CD-ROM drive. Allows CD-R burning from
>   within the guest.
>
> A CD-ROM prior to this patch would always provide an 'atapi-cdrom' is
> modeled as:
>   <disk type='block' device='cdrom'>
>     <source dev='/dev/scd0'/>
>     <target dev='hda' bus='ide'/>
>     <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>   </disk>
>
> This patch allows the 'device' attribute to be set to 'lun' for a raw
> acccess CD-ROM such as:
>   <disk type='block' device='lun'>
>     <source dev='/dev/scd0'/>
>     <target dev='hda' bus='ide'/>
>     <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>   </disk>
> ---
>  docs/formatdomain.html.in                          |  3 +--
>  src/vmx/vmx.c                                      | 24 +++++++++++++++-------
>  tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx |  5 +++++
>  tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 24 ++++++++++++++++++++++
>  .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx  |  6 ++++++
>  .../vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml  | 24 ++++++++++++++++++++++
>  tests/vmx2xmltest.c                                |  2 ++
>  tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx | 13 ++++++++++++
>  tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml | 14 +++++++++++++
>  .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx  | 14 +++++++++++++
>  .../xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml  | 14 +++++++++++++
>  tests/xml2vmxtest.c                                |  2 ++
>  12 files changed, 136 insertions(+), 9 deletions(-)
>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx
>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml
>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.vmx
>  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml
>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.vmx
>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-ide-raw-device.xml
>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.vmx
>  create mode 100644 tests/xml2vmxdata/xml2vmx-cdrom-scsi-raw-device.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index cce179d..b7a5c12 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1597,8 +1597,7 @@
>              defaulting to "disk".
>              <p>
>              Using "lun" (<span class="since">since 0.9.10</span>) is only
> -            valid when type is "block" and the target element's "bus"
> -            attribute is "virtio", and behaves identically to "disk",
> +            valid when type is "block", and behaves identically to "disk",
>              except that generic SCSI commands from the guest are accepted
>              and passed through to the physical device. Also note that
>              device='lun' will only be recognized for actual raw devices,
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index f5cb9fe..bcbb997 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -1936,7 +1936,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
>                  virDomainDiskDefPtr *def)
>  {
>      /*
> -     *          device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM}
> +     *          device = {VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_DEVICE_LUN}

Very long line.

Not quite sure what the correct style would be here. Just wrap the LUN or wrap both the CDROM and LUN? The rest of the source code has there lines on 1 long line.

 

ACK with those two nits fixed.

Michal

--
Doug Goldstein

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