Reorganize metadata parsing so it happens only once.

diff --git a/lib/diskimage_lvm2.ml b/lib/diskimage_lvm2.ml
index da61600..0a9b74a 100644 (file)
--- a/lib/diskimage_lvm2.ml
+++ b/lib/diskimage_lvm2.ml
@@ -33,6 +33,9 @@ let id = "LVM2"
 let sector_size_int = 512
 let sector_size = ~^sector_size_int
 
+(*let attach_private_data, get_private_data =
+  private_data_functions (fun {lvm_cb = {lvm_cb_uq = u}} -> u)*)
+
 (*----------------------------------------------------------------------*)
 (* Block device which can do linear maps, same as the kernel dm-linear.c *)
 class linear_map_device name extent_size segments =
@@ -104,7 +107,7 @@ end
 (* Probe to see if it's an LVM2 PV. *)
 let rec probe dev =
   try
-    let uuid, _ = read_pv_label dev in
+    let uuid, _, _ = read_pv_label dev in
     if !debug then
       eprintf "LVM2 detected PV UUID %s\n%!" uuid;
     { pv_cb = callbacks (); pv_uuid = uuid; pv_dev = dev }
@@ -120,8 +123,6 @@ and read_pv_label dev =
    *)
   let bits = dev#read_bitstring ~^0 (~^9 *^ sector_size) in
 
-  (*Bitmatch.hexdump_bitstring stdout bits;*)
-
   bitmatch bits with
   | {
       (* sector 0 *)
@@ -131,7 +132,7 @@ and read_pv_label dev =
       "LABELONE" : 64 : string;                (* "LABELONE" *)
       _ : 128 : bitstring;             (* Seems to contain something. *)
       "LVM2 001" : 64 : string;                (* "LVM2 001" *)
-      uuid : 256 : string;             (* UUID *)
+      uuid : 256 : string;             (* PV UUID *)
       endsect : (sector_size_int-64)*8 : bitstring;(* to end of second sector *)
 
       (* sectors 2-7 *)
@@ -147,34 +148,23 @@ and read_pv_label dev =
       (* Metadata offset is relative to end of PV label. *)
       let metadata_offset = Int63.of_int32 metadata_offset +^ ~^0x1000 in
       (* Metadata length appears to include the trailing \000 which
-       * we don't want.
+       * we don't want, so subtract 1 to get the true length.
        *)
       let metadata_length = Int63.of_int32 metadata_length -^ ~^1 in
 
-      let metadata = read_metadata dev metadata_offset metadata_length in
+      (* Check the metadata offset and length are sensible. *)
+      if metadata_offset <= ~^0x1200 || metadata_offset >= dev#size
+       || metadata_length <= ~^0
+       || metadata_offset +^ metadata_length >= dev#size then
+         invalid_arg "LVM2: read_metadata: bad metadata offset or length";
 
-      uuid, metadata
+      uuid, metadata_offset, metadata_length
 
   | { _ } ->
       invalid_arg
        (sprintf "LVM2: read_pv_label: %s: not an LVM2 physical volume"
           dev#name)
 
-and read_metadata dev offset len =
-  if !debug then
-    eprintf "metadata: offset %s len %s bytes\n%!"
-      (Int63.to_string offset) (Int63.to_string len);
-
-  (* Check the offset and length are sensible. *)
-  if offset <= ~^0x1200 || offset >= dev#size
-    || len <= ~^0 || offset +^ len >= dev#size then
-      invalid_arg "LVM2: read_metadata: bad metadata offset or length";
-
-  (* If it is outside the disk boundaries, this will throw an exception,
-   * otherwise it will read and return the metadata string.
-   *)
-  dev#read offset len
-
 (*----------------------------------------------------------------------*)
 (* We are passed a list of devices which we previously identified
  * as PVs belonging to us.  From these produce a list of all LVs
@@ -182,23 +172,29 @@ and read_metadata dev offset len =
  * what is on these LVs - that will be done in the main code.
  *)
 and list_lvs pvs =
-  (* Read the UUID and metadata (again) from each device to end up with
-   * an assoc list of PVs, keyed on the UUID.
-   *
-   * XXX We've already read this - we should save it in the pv struct.
+  (* Read the PV label (again) for each PV, and this time also
+   * read out the metadata, which is a big block of text.
    *)
-  let pvs = List.map (
+  let pvsmap = List.map (
     fun { pv_dev = dev } ->
-      let uuid, metadata = read_pv_label dev in
+      let uuid, metadata_offset, metadata_length = read_pv_label dev in
+      let metadata = dev#read metadata_offset metadata_length in
+
+      if !debug then
+       eprintf "list_lvs: metadata for PV %s (offset %s len %s):\n%s\n%!"
+         dev#name
+         (Int63.to_string metadata_offset) (Int63.to_string metadata_length)
+         metadata;
+
       (uuid, (metadata, dev))
   ) pvs in
 
   (* Parse the metadata using the external lexer/parser. *)
-  let pvs = List.map (
+  let pvsmap = List.map (
     fun (uuid, (metadata, dev)) ->
       uuid, (Diskimage_lvm2_lexer.parse_lvm2_metadata_from_string metadata,
             dev)
-  ) pvs in
+  ) pvsmap in
 
   (* Print the parsed metadata. *)
   if !debug then
@@ -206,7 +202,7 @@ and list_lvs pvs =
       fun (uuid, (metadata, dev)) ->
        eprintf "metadata for PV UUID %s on %s:\n" uuid dev#name;
        output_metadata stderr metadata
-    ) pvs;
+    ) pvsmap;
 
   (* Scan for volume groups.  The first entry in the metadata
    * appears to be the volume group name.  This gives us a
@@ -218,7 +214,7 @@ and list_lvs pvs =
       | pvuuid, (((vgname, Metadata vgmeta) :: _), dev) ->
          Some (vgname, (pvuuid, vgmeta))
       | _ -> None
-    ) pvs in
+    ) pvsmap in
 
   let cmp ((a:string),_) ((b:string),_) = compare a b in
   let vgnames = List.sort ~cmp vgnames in
@@ -306,7 +302,7 @@ and list_lvs pvs =
                let pvuuid = canonical_uuid pvuuid in
 
                (* Get the underlying physical device. *)
-               let _, dev = List.assoc pvuuid pvs in
+               let _, dev = List.assoc pvuuid pvsmap in
 
                (* Construct a PV device. *)
                let pe_start = get_int63 "pe_start" meta in
@@ -420,6 +416,8 @@ and list_lvs pvs =
     flush stderr
   );
 
+(*  List.iter (fun pv -> attach_private_data pv vgs) pvs; *)
+
   (* Finally we can set up devices for the LVs. *)
   let lvs =
     List.map (
@@ -450,9 +448,8 @@ and list_lvs pvs =
   (* Return the list of LV devices. *)
   lvs
 
-(* XXX We don't currently have enough information in the PV
- * structure to determine quickly which blocks are used.  Need
- * to store the parsed metadata in the structure ...
+(* XXX We need to reparse the metadata in a different way in
+ * order to calculate this.  Need to generalize metadata handling.
  *)
 and offset_is_free _ _ = false