Re: [libvirt] [PATCH] util: allow creation of file with virFileWriteStr

2010-12-04 Thread Matthias Bolte
2010/12/3 Eric Blake :
> Making this change will allow the future patches to use
> virFileWriteStr to create a file, rather than its current limitation
> of only working on pre-existing files.
>
> * src/util/util.h (virFileWriteStr): Alter signature.
> * src/util/util.c (virFileWriteStr): Allow file creation.
> * src/network/bridge_driver.c (networkEnableIpForwarding)
> (networkDisableIPV6): Adjust clients.
> * src/node_device/node_device_driver.c
> (nodeDeviceVportCreateDelete): Likewise.
> * src/util/cgroup.c (virCgroupSetValueStr): Likewise.
> * src/util/pci.c (pciBindDeviceToStub, pciUnBindDeviceFromStub):
> Likewise.
> Based on a report from Jean-Baptiste Rouault.
> ---
>
>> Alternatively, I only counted 16 existing users of virFileWriteStr; and
>> this is an internal API.  We could easily rewrite all clients to always
>> pass a third parameter, and change the signature of virFileWriteStr to
>> require a mode_t argument.  Hmm; some of those clients are writing to
>> kernel files that should always exist (/proc/sys/net/ipv4/ip_forward,
>> for example), where it's tough to justify what we would pass as a mode_t
>> argument.  So maybe pass mode==0 as a sentinel to require a pre-existing
>> file
>
> How does this look?  Admittedly, all existing uses were okay with a
> mode parameter of 0; and I haven't yet seen your patch that would
> use a non-zero mode, but this still makes more sense to me.
>
> Oh, and VIR_FORCE_CLOSE preserves errno, so I was able to simplify
> virFileWriteStr in the process.
>
>  src/network/bridge_driver.c          |    8 
>  src/node_device/node_device_driver.c |    2 +-
>  src/util/cgroup.c                    |    2 +-
>  src/util/pci.c                       |   16 
>  src/util/util.c                      |   13 -
>  src/util/util.h                      |    3 ++-
>  6 files changed, 24 insertions(+), 20 deletions(-)
>

ACK.

Matthias

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

[libvirt] [PATCH] util: allow creation of file with virFileWriteStr

2010-12-02 Thread Eric Blake
Making this change will allow the future patches to use
virFileWriteStr to create a file, rather than its current limitation
of only working on pre-existing files.

* src/util/util.h (virFileWriteStr): Alter signature.
* src/util/util.c (virFileWriteStr): Allow file creation.
* src/network/bridge_driver.c (networkEnableIpForwarding)
(networkDisableIPV6): Adjust clients.
* src/node_device/node_device_driver.c
(nodeDeviceVportCreateDelete): Likewise.
* src/util/cgroup.c (virCgroupSetValueStr): Likewise.
* src/util/pci.c (pciBindDeviceToStub, pciUnBindDeviceFromStub):
Likewise.
Based on a report from Jean-Baptiste Rouault.
---

> Alternatively, I only counted 16 existing users of virFileWriteStr; and
> this is an internal API.  We could easily rewrite all clients to always
> pass a third parameter, and change the signature of virFileWriteStr to
> require a mode_t argument.  Hmm; some of those clients are writing to
> kernel files that should always exist (/proc/sys/net/ipv4/ip_forward,
> for example), where it's tough to justify what we would pass as a mode_t
> argument.  So maybe pass mode==0 as a sentinel to require a pre-existing
> file

How does this look?  Admittedly, all existing uses were okay with a
mode parameter of 0; and I haven't yet seen your patch that would
use a non-zero mode, but this still makes more sense to me.

Oh, and VIR_FORCE_CLOSE preserves errno, so I was able to simplify
virFileWriteStr in the process.

 src/network/bridge_driver.c  |8 
 src/node_device/node_device_driver.c |2 +-
 src/util/cgroup.c|2 +-
 src/util/pci.c   |   16 
 src/util/util.c  |   13 -
 src/util/util.h  |3 ++-
 6 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 54890f9..62639e4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1024,7 +1024,7 @@ networkReloadIptablesRules(struct network_driver *driver)
 static int
 networkEnableIpForwarding(void)
 {
-return virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n");
+return virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0);
 }

 #define SYSCTL_PATH "/proc/sys"
@@ -1045,7 +1045,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }

-if (virFileWriteStr(field, "1") < 0) {
+if (virFileWriteStr(field, "1", 0) < 0) {
 virReportSystemError(errno,
  _("cannot enable %s"), field);
 goto cleanup;
@@ -1057,7 +1057,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }

-if (virFileWriteStr(field, "0") < 0) {
+if (virFileWriteStr(field, "0", 0) < 0) {
 virReportSystemError(errno,
  _("cannot disable %s"), field);
 goto cleanup;
@@ -1069,7 +1069,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }

-if (virFileWriteStr(field, "1") < 0) {
+if (virFileWriteStr(field, "1", 0) < 0) {
 virReportSystemError(errno,
  _("cannot enable %s"), field);
 goto cleanup;
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 448cfd3..a6c6042 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -454,7 +454,7 @@ nodeDeviceVportCreateDelete(const int parent_host,
 goto cleanup;
 }

-if (virFileWriteStr(operation_path, vport_name) == -1) {
+if (virFileWriteStr(operation_path, vport_name, 0) == -1) {
 virReportSystemError(errno,
  _("Write of '%s' to '%s' during "
"vport create/delete failed"),
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 2758a8f..3ba6325 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -288,7 +288,7 @@ static int virCgroupSetValueStr(virCgroupPtr group,
 return rc;

 VIR_DEBUG("Set value '%s' to '%s'", keypath, value);
-rc = virFileWriteStr(keypath, value);
+rc = virFileWriteStr(keypath, value, 0);
 if (rc < 0) {
 DEBUG("Failed to write value '%s': %m", value);
 rc = -errno;
diff --git a/src/util/pci.c b/src/util/pci.c
index d38cefa..095ad3f 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -849,7 +849,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
  * bound by the stub.
  */
 pciDriverFile(path, sizeof(path), driver, "new_id");
-if (virFileWriteStr(path, dev->id) < 0) {
+if (virFileWriteStr(path, dev->id, 0) < 0) {
 virReportSystemError(errno,
  _("Failed to add PCI device ID '%s' to %s"),
  dev->id, driver);
@@ -862,7 +862,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
  * your root filesystem.
  */
 pciDev