Re: [libvirt] [PATCH 1/8] util: Prepare helpers for unpriv_sgio setting
On 12/13/2012 12:05 PM, Osier Yang wrote:
"virGetDevice{Major,Minor}" could be used across the
sources,
but it doesn't relate with this series, and could be done later.
* src/util/util.h: (Declare virGetDevice{Major,Minor}, and
You generally want both values in one go; calling stat() twice because
you have two functions is not only a waste, but a racy window (if
someone is modifying the pathname in the meantime).
+static char *
+virGetUnprivSGIOSysfsPath(const char *path)
+{
+ int major, minor;
+ char *sysfs_path = NULL;
+
+ if ((major = virGetDeviceMajor(path)) < 0) {
+ virReportSystemError(-major,
+ _("Unable to get major number of device
'%s'"),
+ path);
+ return NULL;
+ }
+
+ if ((minor = virGetDeviceMinor(path)) < 0) {
+ virReportSystemError(-minor,
+ _("Unable to get minor number of device
'%s'"),
+ path);
+ return NULL;
+ }
+
+ if (virAsprintf(&sysfs_path,
"/sys/dev/block/%d:%d/queue/unpriv_sgio",
+ major, minor) < 0) {
This is hard-coded to probe the actual kernel. If you instead make it
use a configurable prefix, then we could default to the kernel path, but
also allow our testsuite to pass in a prefix from the testsuite, so that
we can test this functionality even on kernels that don't support the
feature (similar to how we have tests/nodeinfodata for faked cpu and
node information). I'm not yet sure whether we'll need to fake this
information in any of our tests, but it's food for thought.
+int
+virSetDeviceUnprivSGIO(const char *path,
+ int unpriv_sgio)
Is this value only ever going to be 0 or 1? If so, a bool might be more
appropriate.
+{
+ char *sysfs_path = NULL;
+ char *val = NULL;
+ int ret = -1;
+ int rc = -1;
+
+ if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path)))
+ return -1;
+
+ if (!virFileExists(sysfs_path)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("unpriv_sgio is not supported by this kernel"));
+ goto cleanup;
+ }
+
+ if (virAsprintf(&val, "%d", unpriv_sgio) < 0) {
If indeed this is bool, then you could avoid the virAsprintf,
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) {
and instead write a single '0' or '1' with less malloc pressure.
+ virReportSystemError(-rc, _("failed to set %s"),
sysfs_path);
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ VIR_FREE(sysfs_path);
+ VIR_FREE(val);
+ return ret;
+}
+
+int
+virGetDeviceUnprivSGIO(const char *path,
+ int *unpriv_sgio)
Again, to we expect the kernel to ever return more than 0 or 1? Would
this interface be any simpler as:
/* -1 for error, 0 for off, 1 for on */
int virGetDeviceUnprivSGIO(const char *path)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)