On Jan 2, 2015, at 9:38 AM, Stefan Hajnoczi wrote: > On Sun, Dec 28, 2014 at 04:18:38PM -0500, Programmingkid wrote: > > Suggestion for concise subject line: > > block/raw-posix: fix raw_getlength() for host CD-ROMs on Mac > >> This patch fixes the problem with raw_getlength() on Mac OS X so that it >> actually calculates the correct size of a volume. It has been updated to fix >> certain coding style issues. Booting and using a real CD in QEMU works >> again. > > Plain text emails are usually wrapped at 72 characters. It makes it > easier to read the git log if you wrap lines.
On my email program, the lines just wrap at the width of the window they are in. Maybe there is a feature in your email program to wrap lines. > > Good job braving the nasty nest of #ifdefs in raw_getlength() :). The > code change looks good. Minor changes below - normally I'd make them > while merging your patch but I don't compile QEMU on Mac so I can't > compile test it. Please send a new version of this patch. Thank you for the kind remarks. I can't believe some of the code in that file was actually committed. I think someone should remove most of the #ifdefs and pretty up the file. I would probably do something like mac_raw_getlength(), windows_raw_getlength(), linux_raw_getlength(), sun_raw_getlength()... Then just use the correct function for the host. > >> >> signed-off-by: John Arbuckle <programmingk...@gmail.com> > > Not sure if tags are case-sensistive but it is usually written as > "Signed-off-by:". The git -s option does this for you, that's usually > more convenient than typing it out manually. Ok, that will be easy to fix. > >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index e51293a..a090c9c 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -1312,7 +1312,24 @@ again: >> if (size == 0) >> #endif >> #if defined(__APPLE__) && defined(__MACH__) >> - size = LLONG_MAX; >> + { >> + uint64_t sectors = 0; >> + uint32_t sectorSize = 0; > > Please follow QEMU coding style and use lower_case_with_underscore > variable names. Fixed. > >> + int ret; > > No need to shadow the local variable that was already declared at the > top of the function. Please drop this. Done. Thank you for reviewing my patch.