Re: [libvirt] [PATCH] Clarify why some controllers don't generate -device string in QEMU driver

2010-02-01 Thread Daniel Veillard 
On Thu, Jan 28, 2010 at 12:53:56PM +, Matthew Booth wrote:
 The QEMU driver contained code to generate a -device string for piix4-ide, but
 wasn't using it. This change removes this string generation. It also adds a
 comment explaining why IDE and FDC controllers don't generate -device strings.
 
 The change also generates an error if a sata controller is specified for a 
 QEMU
 domain, as this isn't supported.
 
 * src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
 qemuBuildControllerDevStr(). Ignore IDE and FDC
 controllers. Error if SATA controller is discovered. 
 Add
 comments.
 ---
  src/qemu/qemu_conf.c |   26 ++
  1 files changed, 18 insertions(+), 8 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index f4a6c08..3b7793f 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr 
 def)
  virBufferVSprintf(buf, ,id=scsi%d, def-idx);
  break;
  
 +/* We always get an IDE controller, whether we want it or not. */
  case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
 -virBufferAddLit(buf, piix4-ide);
 -virBufferVSprintf(buf, ,id=ide%d, def-idx);
 -break;
 -
  default:
  goto error;
  }
 @@ -3141,16 +3138,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
  
  if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
  for (i = 0 ; i  def-ncontrollers ; i++) {
 -char *scsi;
 -if (def-controllers[i]-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
 +virDomainControllerDefPtr cont = def-controllers[i];
 +
 +/* We don't add an explicit IDE or FD controller because the
 + * provided PIIX4 device already includes one. It isn't possible 
 to
 + * remove the PIIX4. */
 +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_IDE ||
 +cont-type == VIR_DOMAIN_CONTROLLER_TYPE_FDC)
  continue;
  
 +/* QEMU doesn't implement a SATA driver */
 +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
 +qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
 + %s, _(SATA is not supported with this 
 QEMU binary));
 +goto error;
 +}
 +
  ADD_ARG_LIT(-device);
  
 -if (!(scsi = qemuBuildControllerDevStr(def-controllers[i])))
 +char *devstr;
 +if (!(devstr = qemuBuildControllerDevStr(def-controllers[i])))
  goto no_memory;
  
 -ADD_ARG(scsi);
 +ADD_ARG(devstr);
  }
  }
  

  Okay, pushed, thanks !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

[libvirt] [PATCH] Clarify why some controllers don't generate -device string in QEMU driver

2010-01-28 Thread Matthew Booth 
The QEMU driver contained code to generate a -device string for piix4-ide, but
wasn't using it. This change removes this string generation. It also adds a
comment explaining why IDE and FDC controllers don't generate -device strings.

The change also generates an error if a sata controller is specified for a QEMU
domain, as this isn't supported.

* src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
qemuBuildControllerDevStr(). Ignore IDE and FDC
controllers. Error if SATA controller is discovered. Add
comments.
---
 src/qemu/qemu_conf.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f4a6c08..1c4f326 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
 virBufferVSprintf(buf, ,id=scsi%d, def-idx);
 break;
 
+/* We always get an IDE controller, whether we want it or not. */
 case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
-virBufferAddLit(buf, piix4-ide);
-virBufferVSprintf(buf, ,id=ide%d, def-idx);
-break;
-
 default:
 goto error;
 }
@@ -3141,16 +3138,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
 
 if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
 for (i = 0 ; i  def-ncontrollers ; i++) {
-char *scsi;
-if (def-controllers[i]-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+/* We don't add an explicit IDE controller because the provided
+ * PIIX4 device already includes one. It isn't possible to remove
+ * the PIIX4. */
+if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_IDE)
 continue;
 
 ADD_ARG_LIT(-device);
 
-if (!(scsi = qemuBuildControllerDevStr(def-controllers[i])))
+char *devstr;
+if (!(devstr = qemuBuildControllerDevStr(def-controllers[i])))
 goto no_memory;
 
-ADD_ARG(scsi);
+ADD_ARG(devstr);
 }
 }
 
-- 
1.6.6

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

[libvirt] [PATCH] Clarify why some controllers don't generate -device string in QEMU driver

2010-01-28 Thread Matthew Booth 
The QEMU driver contained code to generate a -device string for piix4-ide, but
wasn't using it. This change removes this string generation. It also adds a
comment explaining why IDE and FDC controllers don't generate -device strings.

The change also generates an error if a sata controller is specified for a QEMU
domain, as this isn't supported.

* src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
qemuBuildControllerDevStr(). Ignore IDE and FDC
controllers. Error if SATA controller is discovered. Add
comments.
---
 src/qemu/qemu_conf.c |   26 ++
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f4a6c08..3b7793f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
 virBufferVSprintf(buf, ,id=scsi%d, def-idx);
 break;
 
+/* We always get an IDE controller, whether we want it or not. */
 case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
-virBufferAddLit(buf, piix4-ide);
-virBufferVSprintf(buf, ,id=ide%d, def-idx);
-break;
-
 default:
 goto error;
 }
@@ -3141,16 +3138,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
 
 if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
 for (i = 0 ; i  def-ncontrollers ; i++) {
-char *scsi;
-if (def-controllers[i]-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+virDomainControllerDefPtr cont = def-controllers[i];
+
+/* We don't add an explicit IDE or FD controller because the
+ * provided PIIX4 device already includes one. It isn't possible to
+ * remove the PIIX4. */
+if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_IDE ||
+cont-type == VIR_DOMAIN_CONTROLLER_TYPE_FDC)
 continue;
 
+/* QEMU doesn't implement a SATA driver */
+if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+ %s, _(SATA is not supported with this QEMU 
binary));
+goto error;
+}
+
 ADD_ARG_LIT(-device);
 
-if (!(scsi = qemuBuildControllerDevStr(def-controllers[i])))
+char *devstr;
+if (!(devstr = qemuBuildControllerDevStr(def-controllers[i])))
 goto no_memory;
 
-ADD_ARG(scsi);
+ADD_ARG(devstr);
 }
 }
 
-- 
1.6.6

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

Re: [libvirt] [PATCH] Clarify why some controllers don't generate -device string in QEMU driver

2010-01-28 Thread Daniel P. Berrange 
On Thu, Jan 28, 2010 at 12:53:56PM +, Matthew Booth wrote:
 The QEMU driver contained code to generate a -device string for piix4-ide, but
 wasn't using it. This change removes this string generation. It also adds a
 comment explaining why IDE and FDC controllers don't generate -device strings.
 
 The change also generates an error if a sata controller is specified for a 
 QEMU
 domain, as this isn't supported.
 
 * src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
 qemuBuildControllerDevStr(). Ignore IDE and FDC
 controllers. Error if SATA controller is discovered. 
 Add
 comments.
 ---
  src/qemu/qemu_conf.c |   26 ++
  1 files changed, 18 insertions(+), 8 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index f4a6c08..3b7793f 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr 
 def)
  virBufferVSprintf(buf, ,id=scsi%d, def-idx);
  break;
  
 +/* We always get an IDE controller, whether we want it or not. */
  case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
 -virBufferAddLit(buf, piix4-ide);
 -virBufferVSprintf(buf, ,id=ide%d, def-idx);
 -break;
 -
  default:
  goto error;
  }
 @@ -3141,16 +3138,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
  
  if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
  for (i = 0 ; i  def-ncontrollers ; i++) {
 -char *scsi;
 -if (def-controllers[i]-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
 +virDomainControllerDefPtr cont = def-controllers[i];
 +
 +/* We don't add an explicit IDE or FD controller because the
 + * provided PIIX4 device already includes one. It isn't possible 
 to
 + * remove the PIIX4. */
 +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_IDE ||
 +cont-type == VIR_DOMAIN_CONTROLLER_TYPE_FDC)
  continue;
  
 +/* QEMU doesn't implement a SATA driver */
 +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
 +qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
 + %s, _(SATA is not supported with this 
 QEMU binary));
 +goto error;
 +}
 +
  ADD_ARG_LIT(-device);
  
 -if (!(scsi = qemuBuildControllerDevStr(def-controllers[i])))
 +char *devstr;
 +if (!(devstr = qemuBuildControllerDevStr(def-controllers[i])))
  goto no_memory;
  
 -ADD_ARG(scsi);
 +ADD_ARG(devstr);
  }
  }

ACK, looks fine now

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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