[libvirt] [PATCH 0/5] RFC: VMX parsing improvements
A user came into #virt the other day and was trying to get libvirtd to work with VMWare Fusion 5, which is basically the Mac OS X version of VMWare Workstation. In helping him out I noticed a few limitations of our VMX parser so I've added support through this patchset. However I came across the fact that we only support 2 types of CD-ROMs instead of the 3 types that VMWare has which leads me to believe we need to extend our XML syntax to define VMWare VMs so I'm asking for feedback and input. CD-ROM types: - cdrom-image = Provides the ISO to the domain - atapi-cdrom = Provides a NEC emulated ATAPI CD-ROM - cdrom-raw = Passthru for a host CD-ROM drive Currently we model 'atapi-cdrom' as (example uses IDE): <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> I was thinking of making that: <disk type='block' device='cdrom'> <driver name='vmware' type='atapi'/> <source dev='/dev/scd0'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> And making 'cdrom-raw' as: <disk type='block' device='cdrom'> <driver name='vmware' type='raw'/> <source dev='/dev/scd0'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> The documentation is a bit ambiguous if I should instead do: <driver name='atapi'/> and <driver name='raw'/> Which is why the request for input. The other reason I'm asking for input is I'm not quite sure what we want to do about CD-ROMs marked as 'auto detect', which is what VMWare Workstation and VMWare Fusion according to their documentation use as the default for 'cdrom-raw' device type. It will either at boot time connect your CD-ROM drive to the VM or dynamically when the user requests it. In this patchset I've modeled it as an opened CD-ROM drive and expect that VMWare will give us a tray closed event or CD-ROM attached event when it gets attached. I unfortunately don't have VMWare and have not used their products in many years so I can't test this myself. Doug Goldstein (5): VMX: Improve disk parse error for unknown values VMX: Add cdrom-raw dev type from VMWare Fusion VMX: Add support for 'auto detect' fileNames VMX: Some serial ports are not actually connected VMX: Add a VMWare Fusion 5 configuration for tests src/vmx/vmx.c | 41 ++++++---- .../vmx2xml-cdrom-ide-raw-auto-detect.vmx | 5 ++ .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 23 ++++++ tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx | 5 ++ tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 24 ++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 38 ++++++++++ tests/vmx2xmltest.c | 4 + 8 files changed, 214 insertions(+), 14 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml 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-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml -- 1.8.1.5
Previously the error message showed the following: error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName' This left the user unsure if it was a CD-ROM or a disk device that they needed to fix. Now the error shows: error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName' for device type 'cdrom-raw' Which should hopefully make it easier to see the issue with the VMX configuration. --- src/vmx/vmx.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 28a6f10..35afe26 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2184,8 +2184,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto ignore; } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid or not yet handled value '%s' for VMX entry " - "'%s'"), fileName, fileName_name); + _("Invalid or not yet handled value '%s' " + "for VMX entry '%s' for device type '%s'"), + fileName, fileName_name, + deviceType ? deviceType : "unknown"); goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) { @@ -2220,8 +2222,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con fileName = NULL; } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid or not yet handled value '%s' for VMX entry " - "'%s'"), fileName, fileName_name); + _("Invalid or not yet handled value '%s' " + "for VMX entry '%s' for device type '%s'"), + fileName, fileName_name, + deviceType ? deviceType : "unknown"); goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -2248,8 +2252,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con fileName = NULL; } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid or not yet handled value '%s' for VMX entry " - "'%s'"), fileName, fileName_name); + _("Invalid or not yet handled value '%s' " + "for VMX entry '%s' for device type '%s'"), + fileName, fileName_name, + deviceType ? deviceType : "unknown"); goto cleanup; } } else { -- 1.8.1.5
On 08/17/2013 01:56 PM, Doug Goldstein wrote:
Previously the error message showed the following:
error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName'
This left the user unsure if it was a CD-ROM or a disk device that they needed to fix. Now the error shows:
error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName' for device type 'cdrom-raw'
Which should hopefully make it easier to see the issue with the VMX configuration. --- src/vmx/vmx.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Aug 21, 2013 at 4:46 PM, Eric Blake <eblake@redhat.com> wrote:
On 08/17/2013 01:56 PM, Doug Goldstein wrote:
Previously the error message showed the following:
error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName'
This left the user unsure if it was a CD-ROM or a disk device that they needed to fix. Now the error shows:
error: internal error: Invalid or not yet handled value 'auto detect' for VMX entry 'ide0:0.fileName' for device type 'cdrom-raw'
Which should hopefully make it easier to see the issue with the VMX configuration. --- src/vmx/vmx.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
ACK.
Thanks. I pushed this but the rest of the series has some more work given the feedback from IRC. -- Doug Goldstein
According to VMWare's documentation 'cdrom-raw' is an acceptable value for deviceType for a CD-ROM drive. ide|scsi(n):(n).deviceType = "cdrom-raw|atapi-cdrom|cdrom-image" 'cdrom-raw' appears to pass on your host CD-ROM instead of emulating an ATAPI CD-ROM drive. --- src/vmx/vmx.c | 12 ++++++----- tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx | 5 +++++ tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 24 ++++++++++++++++++++++ tests/vmx2xmltest.c | 1 + 4 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 35afe26..e6b2c74 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2174,12 +2174,13 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (virFileHasSuffix(fileName, ".iso") || - STRCASEEQ(deviceType, "atapi-cdrom")) { + STRCASEEQ(deviceType, "atapi-cdrom") || + STRCASEEQ(deviceType, "cdrom-raw")) { /* * This function was called in order to parse a harddisk device, - * but .iso files and 'atapi-cdrom' devices are for CDROM devices - * only. Just ignore it, another call to this function to parse a - * CDROM device may handle it. + * but .iso files, 'atapi-cdrom', and 'cdrom-raw' devices are for + * CDROM devices only. Just ignore it, another call to this + * function to parse a CDROM device may handle it. */ goto ignore; } else { @@ -2215,7 +2216,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con * handle it. */ goto ignore; - } else if (STRCASEEQ(deviceType, "atapi-cdrom")) { + } else if (STRCASEEQ(deviceType, "atapi-cdrom") || + STRCASEEQ(deviceType, "cdrom-raw")) { (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK; (*def)->src = fileName; diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx new file mode 100644 index 0000000..1648111 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.vmx @@ -0,0 +1,5 @@ +config.version = "8" +virtualHW.version = "4" +ide0:0.present = "true" +ide0:0.deviceType = "cdrom-raw" +ide0:0.fileName = "/dev/scd0" diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml new file mode 100644 index 0000000..a4bf33c --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml @@ -0,0 +1,24 @@ +<domain type='vmware'> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory unit='KiB'>32768</memory> + <currentMemory unit='KiB'>32768</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <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> + <controller type='ide' index='0'/> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index c9616de..1b97520 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -239,6 +239,7 @@ mymain(void) DO_TEST("cdrom-scsi-device", "cdrom-scsi-device"); DO_TEST("cdrom-ide-file", "cdrom-ide-file"); DO_TEST("cdrom-ide-device", "cdrom-ide-device"); + DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device"); DO_TEST("floppy-file", "floppy-file"); DO_TEST("floppy-device", "floppy-device"); -- 1.8.1.5
VMWare Fusion 5 can set the CD-ROM's file name to be 'auto detect' when using the physical drive via 'cdrom-raw' device type. VMWare will then connect to first available host CD-ROM to the virtual machine upon start up according to VMWare documentation. --- src/vmx/vmx.c | 9 +++++++-- .../vmx2xml-cdrom-ide-raw-auto-detect.vmx | 5 +++++ .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 23 ++++++++++++++++++++++ tests/vmx2xmltest.c | 1 + 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index e6b2c74..7eaff65 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2219,9 +2219,14 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con } else if (STRCASEEQ(deviceType, "atapi-cdrom") || STRCASEEQ(deviceType, "cdrom-raw")) { (*def)->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - (*def)->src = fileName; - fileName = NULL; + if (STRCASEEQ(fileName, "auto detect")) { + (*def)->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN; + (*def)->src = NULL; + } else { + (*def)->src = fileName; + fileName = NULL; + } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid or not yet handled value '%s' " diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx new file mode 100644 index 0000000..b2c4caf --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.vmx @@ -0,0 +1,5 @@ +config.version = "8" +virtualHW.version = "4" +ide0:0.present = "true" +ide0:0.deviceType = "cdrom-raw" +ide0:0.fileName = "auto detect" diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml new file mode 100644 index 0000000..feae3c5 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml @@ -0,0 +1,23 @@ +<domain type='vmware'> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory unit='KiB'>32768</memory> + <currentMemory unit='KiB'>32768</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='block' device='cdrom'> + <target dev='hda' bus='ide' tray='open'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 1b97520..12e775c 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -240,6 +240,7 @@ mymain(void) DO_TEST("cdrom-ide-file", "cdrom-ide-file"); DO_TEST("cdrom-ide-device", "cdrom-ide-device"); DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device"); + DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect"); DO_TEST("floppy-file", "floppy-file"); DO_TEST("floppy-device", "floppy-device"); -- 1.8.1.5
Sometimes a serial port might not be actually wired to a device when the user does not have the device connected at power on and we should not consider this a fatal error. --- src/vmx/vmx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 7eaff65..d4ce885 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2704,7 +2704,7 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port, /* vmx:fileName -> def:data.file.path */ if (virVMXGetConfigString(conf, fileName_name, &fileName, false) < 0) { - goto cleanup; + goto ignore; } /* vmx:network.endPoint -> def:data.tcp.listen */ -- 1.8.1.5
A user was having an issue with this specific VMWare Fusion config and he gave me permission to add it as part of our test suite to further expand our VMX test coverage. --- tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx | 88 ++++++++++++++++++++++ tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 38 ++++++++++ tests/vmx2xmltest.c | 2 + 3 files changed, 128 insertions(+) create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx new file mode 100644 index 0000000..ef6af19 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.vmx @@ -0,0 +1,88 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "9" +memsize = "3572" +MemAllowAutoScaleDown = "FALSE" +MemTrimRate = "-1" +displayName = "ATTM_VM" +guestOS = "winxppro" +numvcpus = "2" +sound.present = "TRUE" +sound.filename = "-1" +sound.autodetect = "TRUE" +usb.present = "TRUE" +ethernet0.present = "TRUE" +ethernet0.addressType = "generated" +ethernet0.connectionType = "bridged" +ethernet1.present = "TRUE" +ethernet1.addressType = "generated" +ethernet1.connectionType = "bridged" +scsi0:0.present = "TRUE" +scsi0:0.fileName = "ATTM_VM.vmdk" +pciBridge0.present = "TRUE" +tools.upgrade.policy = "useGlobal" +ehci.present = "TRUE" +ide0:0.present = "TRUE" +ide0:0.autodetect = "TRUE" +ide0:0.filename = "auto detect" +ide0:0.deviceType = "atapi-cdrom" +scsi0.present = "TRUE" +scsi0.virtualDev = "buslogic" +buslogic.noDriver = "FALSE" +extendedConfigFile = "ATTM_VM.vmxf" +virtualHW.productCompatibility = "hosted" +pciBridge4.present = "TRUE" +pciBridge4.virtualDev = "pcieRootPort" +pciBridge4.pciSlotNumber = "21" +pciBridge4.functions = "8" +pciBridge5.present = "TRUE" +pciBridge5.virtualDev = "pcieRootPort" +pciBridge5.pciSlotNumber = "22" +pciBridge5.functions = "8" +pciBridge6.present = "TRUE" +pciBridge6.virtualDev = "pcieRootPort" +pciBridge6.pciSlotNumber = "23" +pciBridge6.functions = "8" +pciBridge7.present = "TRUE" +pciBridge7.virtualDev = "pcieRootPort" +pciBridge7.pciSlotNumber = "24" +pciBridge7.functions = "8" +vmci0.present = "TRUE" +hpet0.present = "TRUE" +usb.vbluetooth.startConnected = "TRUE" +mks.enable3d = "TRUE" +ethernet0.linkStatePropagation.enable = "TRUE" +ethernet1.linkStatePropagation.enable = "TRUE" +ide0:0.startConnected = "FALSE" +ethernet0.generatedAddress = "00:0c:29:3b:64:ea" +ethernet1.generatedAddress = "00:0c:29:3b:64:f4" +vmci0.id = "-952408854" +tools.syncTime = "FALSE" +uuid.location = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea" +uuid.bios = "56 4d 70 88 01 a1 98 32-e7 2b 67 90 c7 3b 64 ea" +cleanShutdown = "TRUE" +replay.supported = "FALSE" +replay.filename = "" +scsi0:0.redo = "" +pciBridge0.pciSlotNumber = "17" +scsi0.pciSlotNumber = "16" +usb.pciSlotNumber = "32" +ethernet0.pciSlotNumber = "33" +ethernet1.pciSlotNumber = "34" +sound.pciSlotNumber = "35" +ehci.pciSlotNumber = "36" +vmci0.pciSlotNumber = "37" +usb:1.present = "TRUE" +ethernet0.generatedAddressOffset = "0" +ethernet1.generatedAddressOffset = "10" +vmotion.checkpointFBSize = "134217728" +usb:1.speed = "2" +usb:1.deviceType = "hub" +usb:1.port = "1" +usb:1.parent = "-1" +floppy0.startConnected = "FALSE" +softPowerOff = "FALSE" +usb:0.present = "TRUE" +usb:0.deviceType = "hid" +usb:0.port = "0" +usb:0.parent = "-1" diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml new file mode 100644 index 0000000..dd8c12a --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml @@ -0,0 +1,38 @@ +<domain type='vmware'> + <name>ATTM_VM</name> + <uuid>564d7088-01a1-9832-e72b-6790c73b64ea</uuid> + <memory unit='KiB'>3657728</memory> + <currentMemory unit='KiB'>3657728</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <source file='[datastore] directory/ATTM_VM.vmdk'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <target dev='hda' bus='ide' tray='open'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='buslogic'/> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:ea'/> + <source bridge=''/> + </interface> + <interface type='bridge'> + <mac address='00:0c:29:3b:64:f4'/> + <source bridge=''/> + </interface> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 12e775c..720ce4c 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -286,6 +286,8 @@ mymain(void) DO_TEST("ws-in-the-wild-1", "ws-in-the-wild-1"); DO_TEST("ws-in-the-wild-2", "ws-in-the-wild-2"); + DO_TEST("fusion-in-the-wild-1", "fusion-in-the-wild-1"); + DO_TEST("annotation", "annotation"); DO_TEST("smbios", "smbios"); -- 1.8.1.5
participants (2)
-
Doug Goldstein -
Eric Blake