Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

(patch) flashcache_load -v option not accepted with cachedev #44

Closed
jimav opened this Issue Dec 20, 2011 · 2 comments

Comments

Projects
None yet
2 participants

jimav commented Dec 20, 2011

There is an option-parsing bug in flashcache_load.c where the -v option prevents passing the optional [cachedev]. The problem is that argc is checked against hard-coded values, without taking into account whether the optional -v arg was passed.

Below is a patch which fixes this. Unfortunately github does not seem to allow attaching files to bugs, and is going to screw with the tabs in the below code block; so you can't just apply this patch to the source. However it is simple enough to extract the needed bits by eye.

--- flashcache_load.c.ORIG  2011-12-20 01:05:04.727129454 -0800
+++ flashcache_load.c   2011-12-20 10:21:05.191387994 -0800
@@ -114,11 +114,20 @@
        }
    }

-   if ((argc < 2) || (argc > 3)) {
+   if (optind >= argc) { 
        usage(pname);
    }
-   
    ssd_devname = argv[optind++];
+
+   if (optind >= argc) {
+       cachedev = NULL;
+   } else {
+       cachedev = argv[optind++];
+   }
+   if (optind < argc) {
+       usage(pname);
+   }
+
    cache_fd = open(ssd_devname, O_RDONLY);
    if (cache_fd < 0) {
        fprintf(stderr, "Failed to open %s\n", ssd_devname);
@@ -137,15 +146,13 @@
        exit(1);
    }

-   if ((strncmp(sb->cache_devname, ssd_devname, DEV_PATHLEN) == 0) && (argc == 2)) {
+   if ((strncmp(sb->cache_devname, ssd_devname, DEV_PATHLEN) == 0) && (cachedev==NULL)) {
        fprintf(stderr, "%s: Upgrading older v2 superblock format, please supply cachedev virtual device name\n", pname);
        usage(pname);
    }

-   // switch to new vdev name if requested by load command
-   if (argc == 3) {
-       cachedev = argv[optind];
-   } else {
+   // retain old vdev name if a new name was not requested
+   if (cachedev == NULL) {
        cachedev = sb->cache_devname;
    }
    disk_devname = sb->disk_devname;

Contributor

mohans commented Dec 21, 2011

I'll get this committed shortly.


From: jimav
reply@reply.github.com

To: Mohan Srinivasan mohan_srinivasan@yahoo.com
Sent: Tue, December 20, 2011 9:53:45 AM
Subject: flashcache flashcache_load -v option not accepted with
cachedev (#44)

There is an option-parsing bug in flashcache_load.c where the -v option prevents
passing the optional [cachedev]. The problem is that argc is checked against
hard-coded values, without taking into account whether the optional -v arg was
passed.

Here is a patch which fixes this (github does not seem to support attaching
files to bugs !?!):

--- flashcache_load.c.ORIG 2011-12-20 01:05:04.727129454 -0800
+++ flashcache_load.c 2011-12-20 09:29:26.934460062 -0800
@@ -114,11 +114,20 @@
}
}

  • if ((argc < 2) || (argc > 3)) {
  •    if (optind >= argc) { 
    
  •            usage(pname);
    
  •    }
    
  • ssd_devname = argv[optind++];
  •    if (optind >= argc) {
    
  •        cachedev = NULL;
    
  •    } else {
    
  •        cachedev = argv[optind++];
    
  •    }
    
  •    if (optind < argc) {
    usage(pname);
    
    }
  • ssd_devname = argv[optind++];

cache_fd = open(ssd_devname, O_RDONLY);
if (cache_fd < 0) {
fprintf(stderr, "Failed to open %s\n", ssd_devname);
@@ -143,9 +152,7 @@
}

// switch to new vdev name if requested by load command

  • if (argc == 3) {
  •    cachedev = argv[optind];
    
  • } else {
  • if (cachedev == NULL) {
    cachedev = sb->cache_devname;
    }
    disk_devname = sb->disk_devname;

Reply to this email directly or view it on GitHub:
facebook#44

jimav commented Dec 21, 2011

Myoriginal patch (shown in your email) was not quite right. There remained some later argc references which needed to be fixed but weren't.

I think the patch currently in the github issue is okay, at facebook#44

-Jim


From: Mohan Srinivasan reply@reply.github.com
To: jimav james_avera@yahoo.com
Sent: Wednesday, December 21, 2011 10:34 AM
Subject: Re: flashcache flashcache_load -v option not accepted with cachedev (#44)

I'll get this committed shortly.


From: jimav
reply@reply.github.com

To: Mohan Srinivasan mohan_srinivasan@yahoo.com
Sent: Tue, December 20, 2011 9:53:45 AM
Subject: flashcache flashcache_load -v option not accepted with
cachedev (#44)

There is an option-parsing bug in flashcache_load.c where the -v option prevents
passing the optional [cachedev]. The problem is that argc is checked against
hard-coded values, without taking into account whether the optional -v arg was
passed.

Here is a patch which fixes this (github does not seem to support attaching
files to bugs !?!):

--- flashcache_load.c.ORIG 2011-12-20 01:05:04.727129454 -0800
+++ flashcache_load.c 2011-12-20 09:29:26.934460062 -0800
@@ -114,11 +114,20 @@
}
}

  • if ((argc < 2) || (argc > 3)) {
  • if (optind >= argc) {
  •    usage(pname);
    
  • }
  • ssd_devname = argv[optind++];
  • if (optind >= argc) {
  •  cachedev = NULL;
    
  • } else {
  •  cachedev = argv[optind++];
    
  • }
  • if (optind < argc) {
    usage(pname);
    }
  • ssd_devname = argv[optind++];

cache_fd = open(ssd_devname, O_RDONLY);
if (cache_fd < 0) {
fprintf(stderr, "Failed to open %s\n", ssd_devname);
@@ -143,9 +152,7 @@
}

// switch to new vdev name if requested by load command

  • if (argc == 3) {
  • cachedev = argv[optind];
  • } else {
  • if (cachedev == NULL) {
    cachedev = sb->cache_devname;
    }
    disk_devname = sb->disk_devname;

Reply to this email directly or view it on GitHub:
facebook#44


Reply to this email directly or view it on GitHub:
facebook#44 (comment)

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment