| Message ID | 3581523B-8162-4CB7-9B21-DFA980E15F97@gmail.com |
|---|---|
| State | New |
| Headers | show |
Programmingkid <programmingkidx@gmail.com> writes: > This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> Subject line is too long, and the body lacks line breaks. Suggest something like: block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work.
On 15 January 2015 at 23:40, Programmingkid <programmingkidx@gmail.com> wrote: > This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > Totally rewritten. > > block/raw-posix.c | 15 ++++++++++++++- > 1 files changed, 14 insertions(+), 1 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index e51293a..a95cfcf 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -1312,7 +1312,20 @@ again: > if (size == 0) > #endif > #if defined(__APPLE__) && defined(__MACH__) > - size = LLONG_MAX; > + { > + uint64_t sectors = 0; > + uint32_t sector_size = 0; > + > + if (ioctl(fd, DKIOCGETBLOCKCOUNT, §ors) == 0 > + && ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) == 0) { > + size = sectors * sector_size; > + } else { > + size = lseek(fd, 0LL, SEEK_END); > + if (size < 0) { > + return -errno; > + } > + } > + } > #else You have the indentation here wrong -- the first '{' should be at the same indent as the 'size = ' line you've deleted; and as Markus says the commit message could be neatened up. Otherwise, code looks good and it passes make check, so Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
On Jan 16, 2015, at 3:22 AM, Markus Armbruster wrote: > Programmingkid <programmingkidx@gmail.com> writes: > >> This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > Subject line is too long, and the body lacks line breaks. Suggest > something like: > > block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD Unfortunately I can't change the subject line after posting the first version of the patch. Others have to know the history of this patch. I will try to include the file name in the future. > > This patch allows Mac OS X to use a real CDROM disc in QEMU. > Testing this patch will require using QEMU v2.2.0 because the > current git version has a bug in it that prevents /dev/cdrom from > being used. "make check" did pass and my Debian boot disc did work. Ok. I just don't know when to break it. I guess I will stick to pinky length.
On 01/19/2015 09:32 AM, Programmingkid wrote: > > On Jan 16, 2015, at 3:22 AM, Markus Armbruster wrote: > >> Programmingkid <programmingkidx@gmail.com> writes: >> >>> This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. >>> >>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> Subject line is too long, and the body lacks line breaks. Suggest >> something like: >> >> block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD > > Unfortunately I can't change the subject line after posting the first version of the patch. Others have to know the history of this patch. I will try to include the file name in the future. Yes, you can. It's a bit trickier, in that you'll probably want to include a mailing list archive link in v8 and explicitly point out that the subject has changed, as well as a followup to v7 that points to the list archive of v8 to help people find the changed name. But there is nothing technically stopping you from renaming a patch to something more appropriate.
Programmingkid <programmingkidx@gmail.com> writes: > On Jan 16, 2015, at 3:22 AM, Markus Armbruster wrote: > >> Programmingkid <programmingkidx@gmail.com> writes: >> >>> This patch allows Mac OS X to use a real CDROM disc in >>> QEMU. Testing this patch will require using QEMU v2.2.0 because the >>> current git version has a bug in it that prevents /dev/cdrom from >>> being used. "make check" did pass and my Debian boot disc did work. >>> >>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> Subject line is too long, and the body lacks line breaks. Suggest >> something like: >> >> block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD > > Unfortunately I can't change the subject line after posting the first > version of the patch. Others have to know the history of this patch. I > will try to include the file name in the future. Oh yes, you can! If you care about patch history, do this: block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- v7: * Totally rewritten. * Subject changed from "block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD" v6: [more history, if you care...] A good commit message is worth a little inconvenience. >> This patch allows Mac OS X to use a real CDROM disc in QEMU. >> Testing this patch will require using QEMU v2.2.0 because the >> current git version has a bug in it that prevents /dev/cdrom from >> being used. "make check" did pass and my Debian boot disc did work. > > Ok. I just don't know when to break it. I guess I will stick to pinky length. I recommend to limit line length to 70 characters.
diff --git a/block/raw-posix.c b/block/raw-posix.c index e51293a..a95cfcf 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1312,7 +1312,20 @@ again: if (size == 0) #endif #if defined(__APPLE__) && defined(__MACH__) - size = LLONG_MAX; + { + uint64_t sectors = 0; + uint32_t sector_size = 0; + + if (ioctl(fd, DKIOCGETBLOCKCOUNT, §ors) == 0 + && ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) == 0) { + size = sectors * sector_size; + } else { + size = lseek(fd, 0LL, SEEK_END); + if (size < 0) { + return -errno; + } + } + } #else size = lseek(fd, 0LL, SEEK_END); if (size < 0) {
This patch allows Mac OS X to use a real CDROM disc in QEMU. Testing this patch will require using QEMU v2.2.0 because the current git version has a bug in it that prevents /dev/cdrom from being used. "make check" did pass and my Debian boot disc did work. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- Totally rewritten. block/raw-posix.c | 15 ++++++++++++++- 1 files changed, 14 insertions(+), 1 deletions(-)