Re: [Xen-devel] [PATCH 2.6.12.6-xen] sysfs attributes for xen

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Comments inlined.

Mike D. Day wrote:

Creates /sys/hypervisor/xen and populates that dir with xen version, changeset, compilation, and capabilities info. Intended for the xen merge tree and later upstream.

<snip>

+    if( 0 ==  (err = subsystem_register(&hypervisor_subsys)) ) {

This formatting is off. You want a space after the if and no space after the (. See CodingStyle. It could be your mailer (it munged the newlines) but it appears you've got 4 space tabs? Linux style is 8.

+ if( ! HYPERVISOR_xen_version(XENVER_extraversion, + extra_version) ) {
+            page[PAGE_SIZE - 1] = 0x00;
+ return snprintf(page, PAGE_SIZE - 1, + "xen-%ld.%ld%s\n",
+                    major, minor, extra_version);

snprintf takes into account the terminating byte so you don't have to.

<snip>

+/* xen compile info */
+static ssize_t xen_compile_show(struct kobject * kobj, + struct attribute * attr, + char * page)
+{
+    struct xen_compile_info info;
+ + if( 0 == HYPERVISOR_xen_version(XENVER_compile_info, &info) ) {
+        page[PAGE_SIZE - 1] = 0x00;
+ return snprintf(page, PAGE_SIZE - 1, + "compiled by %s, using %s, on %s\n", + info.compile_by, + info.compile_date, + info.compiler);
+    }
+    return 0;
+}

I'm not the best person to speak to this but I'm pretty sure sysfs prefers single values per file so you probably want to split this up into compile_by, compile_date, compiler.

+int __init
+sysfs_xen_version_init(void)
+{
+    __label__  out;
+ + struct kset * parent = get_xen_kset();
+    if ( parent != NULL ) {
+        kobject_init(&xen_ver_obj);
+ xen_ver_obj.parent = &parent->kobj; + xen_ver_obj.dentry = parent->kobj.dentry;
+        kobject_get(&parent->kobj);
+        if ( sysfs_create_file(&xen_ver_obj, &xen_ver_attr.attr) )
+            goto out;

I reckon you want to use default attributes here instead of immediately calling sysfs_create_file().

Regards,

Anthony Liguori
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux